* Re: [PATCH 1/2] x86/uprobes: Keep shadow stack in sync for emulated CALLs
From: David Windsor @ 2026-06-27 17:14 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Peter Zijlstra, mhiramat, tglx, mingo, bp, dave.hansen, x86,
shuah, linux-trace-kernel, linux-kselftest, linux-kernel
In-Reply-To: <ajqJNju6MY3B0S2F@redhat.com>
On Tue, Jun 23, 2026 at 9:25 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 06/23, Peter Zijlstra wrote:
> >
> > On Tue, Jun 23, 2026 at 02:52:32PM +0200, Oleg Nesterov wrote:
> > > On 06/22, David Windsor wrote:
> > > >
> > > > --- a/arch/x86/kernel/uprobes.c
> > > > +++ b/arch/x86/kernel/uprobes.c
> > > > @@ -1246,8 +1246,12 @@ 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) {
> > > > + unsigned long retaddr = utask->vaddr + auprobe->defparam.ilen;
> > > > +
> > > > regs->sp += sizeof_long(regs); /* Pop incorrect return address */
> > > > - if (emulate_push_stack(regs, utask->vaddr + auprobe->defparam.ilen))
> > > > + if (emulate_push_stack(regs, retaddr))
> > > > + return -ERESTART;
> > > > + if (shstk_update_last_frame(retaddr))
> > > > return -ERESTART;
> > >
> > > Well, if shstk_update_last_frame() fails after emulate_push_stack(), we should
> > > probably return another error, so that the caller handle_singlestep() will kill
> > > this task?
> >
> > Makes sense, the other user has a force_sig(SIGSEGV) on failure.
>
> Offtopic question... both shstk_update_last_frame() and shstk_push() are only
> used by arch/x86/kernel/uprobes.c. But they are not symmetric in that
> shstk_update_last_frame() returns 0 if !features_enabled(ARCH_SHSTK_SHSTK),
> while shstk_push() returns -ENOTSUPP in this case.
>
> That is why the users can't just do "if (shstk_push(xxx)) ...". This is really
> minor, but perhaps it makes sense to change shstk_push() to return 0 in this
> case too? I don't think -ENOTSUPP is actually useful...
>
Agreed, will send a follow-up patch changing shstk_push() to return 0
rather than -ENOTSUPP.
I'll send v2 shortly with the additional call to force_sig(SIGSEGV) to
balance out the callers.
> Oleg.
>
^ permalink raw reply
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox