Messages in this thread | | | Subject | Re: [PATCH] uprobes/x86: fix detection of 32-bit user mode | From | Sebastian Mayr <> | Date | Mon, 19 Aug 2019 20:40:07 +0200 |
| |
On Sun, 2019-07-28 at 17:26 +0200, Sebastian Mayr wrote: > 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 software interrupt and always returns 'false' (on a 64-bit kernel). > In > consequence this leads to corruption of the process's return address. > > This can be fixed by using user_64bit_mode(), which should always be > correct. > > Signed-off-by: Sebastian Mayr <me@sam.st> > --- > > Please note that I just stumbled over this bug and am not really > familiar with all the internals. So take the patch and, in > particular, > the commit message with a grain of salt. Thanks! > > arch/x86/kernel/uprobes.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c > index 918b5092a85f..d6e261202c6b 100644 > --- a/arch/x86/kernel/uprobes.c > +++ b/arch/x86/kernel/uprobes.c > @@ -508,9 +508,9 @@ struct uprobe_xol_ops { > void (*abort)(struct arch_uprobe *, struct pt_regs *); > }; > > -static inline int sizeof_long(void) > +static inline int sizeof_long(struct pt_regs *regs) > { > - return in_ia32_syscall() ? 4 : 8; > + return user_64bit_mode(regs) ? 8 : 4; > } > > static int default_pre_xol_op(struct arch_uprobe *auprobe, struct > pt_regs *regs) > @@ -521,9 +521,9 @@ static int default_pre_xol_op(struct arch_uprobe > *auprobe, struct pt_regs *regs) > > static int emulate_push_stack(struct pt_regs *regs, unsigned long > val) > { > - 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, &val, sizeof_long())) > + if (copy_to_user((void __user *)new_sp, &val, > sizeof_long(regs))) > return -EFAULT; > > regs->sp = new_sp; > @@ -556,7 +556,7 @@ static int default_post_xol_op(struct arch_uprobe > *auprobe, struct pt_regs *regs > long correction = utask->vaddr - utask->xol_vaddr; > regs->ip += correction; > } 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 (emulate_push_stack(regs, utask->vaddr + auprobe- > >defparam.ilen)) > return -ERESTART; > } > @@ -675,7 +675,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. > * We could also restore ->ip and try to call > branch_emulate_op() again. > */ > - regs->sp += sizeof_long(); > + regs->sp += sizeof_long(regs); > return -ERESTART; > } > > @@ -1056,7 +1056,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe > *auprobe, struct pt_regs *regs) > unsigned long > 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 */ > > if (copy_from_user(&orig_ret_vaddr, (void __user *)regs->sp, > rasize))
Any feedback on this patch would be greatly appreciated.
Thanks, Sebastian
| |