From: Sebastian Mayr <me@sam.st>
To: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] uprobes/x86: fix detection of 32-bit user mode
Date: Mon, 19 Aug 2019 20:40:07 +0200 [thread overview]
Message-ID: <2d8f1744136431b5eb0bda24ea767374d6fde4e5.camel@sam.st> (raw)
In-Reply-To: <20190728152617.7308-1-me@sam.st>
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
next prev parent reply other threads:[~2019-08-19 18:40 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-28 15:26 [PATCH] uprobes/x86: fix detection of 32-bit user mode Sebastian Mayr
2019-08-19 18:40 ` Sebastian Mayr [this message]
2019-08-19 18:43 ` Thomas Gleixner
2019-08-23 23:30 ` Thomas Gleixner
2019-08-23 23:44 ` Thomas Gleixner
2019-08-23 23:57 ` Andy Lutomirski
2019-08-24 0:00 ` Thomas Gleixner
2019-08-24 0:03 ` Thomas Gleixner
2019-08-24 0:13 ` Andy Lutomirski
2019-08-24 0:20 ` Thomas Gleixner
2019-08-26 13:48 ` Thomas Gleixner
2019-08-27 14:00 ` get_unmapped_area && in_ia32_syscall (Was: [PATCH] uprobes/x86: fix detection of 32-bit user mode) Oleg Nesterov
2019-08-27 17:03 ` Dmitry Safonov
2019-08-27 23:40 ` Dmitry Safonov
2019-08-28 11:37 ` Oleg Nesterov
2019-08-26 14:02 ` [tip: x86/urgent] uprobes/x86: Fix detection of 32-bit user mode tip-bot2 for Sebastian Mayr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2d8f1744136431b5eb0bda24ea767374d6fde4e5.camel@sam.st \
--to=me@sam.st \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).