Skip to content

Commit 9212ec7

Browse files
smayr42KAGA-KOKO
authored andcommitted
uprobes/x86: Fix detection of 32-bit user mode
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 abfb949 ("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: 8faaed1 ("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: 0326f5a ("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: 0326f5a ("uprobes/core: Handle breakpoint and singlestep exceptions") Signed-off-by: Sebastian Mayr <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Cc: Masami Hiramatsu <[email protected]> Cc: Dmitry Safonov <[email protected]> Cc: Oleg Nesterov <[email protected]> Cc: Srikar Dronamraju <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected]
1 parent 3e5bedc commit 9212ec7

File tree

1 file changed

+10
-7
lines changed

1 file changed

+10
-7
lines changed

arch/x86/kernel/uprobes.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,12 @@ struct uprobe_xol_ops {
508508
void (*abort)(struct arch_uprobe *, struct pt_regs *);
509509
};
510510

511-
static inline int sizeof_long(void)
511+
static inline int sizeof_long(struct pt_regs *regs)
512512
{
513-
return in_ia32_syscall() ? 4 : 8;
513+
/*
514+
* Check registers for mode as in_xxx_syscall() does not apply here.
515+
*/
516+
return user_64bit_mode(regs) ? 8 : 4;
514517
}
515518

516519
static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
@@ -521,9 +524,9 @@ static int default_pre_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
521524

522525
static int emulate_push_stack(struct pt_regs *regs, unsigned long val)
523526
{
524-
unsigned long new_sp = regs->sp - sizeof_long();
527+
unsigned long new_sp = regs->sp - sizeof_long(regs);
525528

526-
if (copy_to_user((void __user *)new_sp, &val, sizeof_long()))
529+
if (copy_to_user((void __user *)new_sp, &val, sizeof_long(regs)))
527530
return -EFAULT;
528531

529532
regs->sp = new_sp;
@@ -556,7 +559,7 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs
556559
long correction = utask->vaddr - utask->xol_vaddr;
557560
regs->ip += correction;
558561
} else if (auprobe->defparam.fixups & UPROBE_FIX_CALL) {
559-
regs->sp += sizeof_long(); /* Pop incorrect return address */
562+
regs->sp += sizeof_long(regs); /* Pop incorrect return address */
560563
if (emulate_push_stack(regs, utask->vaddr + auprobe->defparam.ilen))
561564
return -ERESTART;
562565
}
@@ -675,7 +678,7 @@ static int branch_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs)
675678
* "call" insn was executed out-of-line. Just restore ->sp and restart.
676679
* We could also restore ->ip and try to call branch_emulate_op() again.
677680
*/
678-
regs->sp += sizeof_long();
681+
regs->sp += sizeof_long(regs);
679682
return -ERESTART;
680683
}
681684

@@ -1056,7 +1059,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
10561059
unsigned long
10571060
arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs)
10581061
{
1059-
int rasize = sizeof_long(), nleft;
1062+
int rasize = sizeof_long(regs), nleft;
10601063
unsigned long orig_ret_vaddr = 0; /* clear high bits for 32-bit apps */
10611064

10621065
if (copy_from_user(&orig_ret_vaddr, (void __user *)regs->sp, rasize))

0 commit comments

Comments
 (0)