Commit 9faf4003 authored by Sebastian Mayr's avatar Sebastian Mayr Committed by Kleber Sacilotto de Souza

uprobes/x86: Fix detection of 32-bit user mode

BugLink: https://bugs.launchpad.net/bugs/1845036

[ Upstream commit 9212ec7d ]

32-bit processes running on a 64-bit kernel are not always detected
correctly, causing the process to crash when uretprobes are installed.

The reason for the crash is that in_ia32_syscall() is used to determine the
process's mode, which only works correctly when called from a syscall.

In the case of uretprobes, however, the function is called from a exception
and always returns 'false' on a 64-bit kernel. In consequence this leads to
corruption of the process's return address.

Fix this by using user_64bit_mode() instead of in_ia32_syscall(), which
is correct in any situation.

[ tglx: Add a comment and the following historical info ]

This should have been detected by the rename which happened in commit

  abfb9498 ("x86/entry: Rename is_{ia32,x32}_task() to in_{ia32,x32}_syscall()")

which states in the changelog:

    The is_ia32_task()/is_x32_task() function names are a big misnomer: they
    suggests that the compat-ness of a system call is a task property, which
    is not true, the compatness of a system call purely depends on how it
    was invoked through the system call layer.
    .....

and then it went and blindly renamed every call site.

Sadly enough this was already mentioned here:

   8faaed1b ("uprobes/x86: Introduce sizeof_long(), cleanup adjust_ret_addr() and
arch_uretprobe_hijack_return_addr()")

where the changelog says:

    TODO: is_ia32_task() is not what we actually want, TS_COMPAT does
    not necessarily mean 32bit. Fortunately syscall-like insns can't be
    probed so it actually works, but it would be better to rename and
    use is_ia32_frame().

and goes all the way back to:

    0326f5a9 ("uprobes/core: Handle breakpoint and singlestep exceptions")

Oh well. 7+ years until someone actually tried a uretprobe on a 32bit
process on a 64bit kernel....

Fixes: 0326f5a9 ("uprobes/core: Handle breakpoint and singlestep exceptions")
Signed-off-by: default avatarSebastian Mayr <me@sam.st>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20190728152617.7308-1-me@sam.stSigned-off-by: default avatarSasha Levin <sashal@kernel.org>
Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent efefb937
...@@ -514,9 +514,12 @@ struct uprobe_xol_ops { ...@@ -514,9 +514,12 @@ struct uprobe_xol_ops {
void (*abort)(struct arch_uprobe *, struct pt_regs *); void (*abort)(struct arch_uprobe *, struct pt_regs *);
}; };
static inline int sizeof_long(void) static inline int sizeof_long(struct pt_regs *regs)
{ {
return is_ia32_task() ? 4 : 8; /*
* Check registers for mode as in_xxx_syscall() does not apply here.
*/
return user_64bit_mode(regs) ? 8 : 4;
} }
static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs) static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
...@@ -527,9 +530,9 @@ static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs) ...@@ -527,9 +530,9 @@ static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
static int push_ret_address(struct pt_regs *regs, unsigned long ip) static int push_ret_address(struct pt_regs *regs, unsigned long ip)
{ {
unsigned long new_sp = regs->sp - sizeof_long(); unsigned long new_sp = regs->sp - sizeof_long(regs);
if (copy_to_user((void __user *)new_sp, &ip, sizeof_long())) if (copy_to_user((void __user *)new_sp, &ip, sizeof_long(regs)))
return -EFAULT; return -EFAULT;
regs->sp = new_sp; regs->sp = new_sp;
...@@ -562,7 +565,7 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs ...@@ -562,7 +565,7 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs
long correction = utask->vaddr - utask->xol_vaddr; long correction = utask->vaddr - utask->xol_vaddr;
regs->ip += correction; regs->ip += correction;
} else if (auprobe->defparam.fixups & UPROBE_FIX_CALL) { } else if (auprobe->defparam.fixups & UPROBE_FIX_CALL) {
regs->sp += sizeof_long(); /* Pop incorrect return address */ regs->sp += sizeof_long(regs); /* Pop incorrect return address */
if (push_ret_address(regs, utask->vaddr + auprobe->defparam.ilen)) if (push_ret_address(regs, utask->vaddr + auprobe->defparam.ilen))
return -ERESTART; return -ERESTART;
} }
...@@ -671,7 +674,7 @@ static int branch_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs) ...@@ -671,7 +674,7 @@ static int branch_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
* "call" insn was executed out-of-line. Just restore ->sp and restart. * "call" insn was executed out-of-line. Just restore ->sp and restart.
* We could also restore ->ip and try to call branch_emulate_op() again. * We could also restore ->ip and try to call branch_emulate_op() again.
*/ */
regs->sp += sizeof_long(); regs->sp += sizeof_long(regs);
return -ERESTART; return -ERESTART;
} }
...@@ -962,7 +965,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) ...@@ -962,7 +965,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
unsigned long unsigned long
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs) arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs)
{ {
int rasize = sizeof_long(), nleft; int rasize = sizeof_long(regs), nleft;
unsigned long orig_ret_vaddr = 0; /* clear high bits for 32-bit apps */ unsigned long orig_ret_vaddr = 0; /* clear high bits for 32-bit apps */
if (copy_from_user(&orig_ret_vaddr, (void __user *)regs->sp, rasize)) if (copy_from_user(&orig_ret_vaddr, (void __user *)regs->sp, rasize))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment