Re: [PATCHv6 perf/core 10/22] uprobes/x86: Add support to optimize uprobes

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Aug 20, 2025 at 05:26:38PM +0000, Edgecombe, Rick P wrote:
> On Wed, 2025-08-20 at 19:12 +0200, Peter Zijlstra wrote:
> > > Are we effectively allowing arbitrary shadow stack push here? 
> > 
> > Yeah, why not? Userspace shadow stacks does not, and cannot, protect
> > from the kernel being funneh. It fully relies on the kernel being
> > trusted. So the kernel doing a shstk_{pop,push}() to make things line up
> > properly shouldn't be a problem.
> 
> Emulating a call/ret should be fine.
> 
> > 
> > > I see we need to be in in_uprobe_trampoline(), but there is no mmap
> > > lock taken, so it's a racy check.
> > 
> > Racy how? Isn't this more or less equivalent to what a normal CALL
> > instruction would do?
> 
> Racy in terms of the "is trampoline" check happening before the write to the
> shadow stack. I was thinking like a TOCTOU thing. The "Allow execution only from
> uprobe trampolines" check is not very strong.
> 
> As for call equivalence, args.retaddr comes from userspace, right?

Yeah. So this whole thing is your random code having a 5 byte nop. And
instead of using INT3 to turn it into #BP, we turn it into "CALL
uprobe_trampoline".

That trampoline looks like:

	push %rcx
	push %r11
	push %rax;
	mov $__NR_uprobe, %rax
	syscall
	pop %rax
	pop %r11
	pop %rcx
	ret

Now, that syscall handler is the one doing shstk_pop/push. But it does
that right along with modifying the normal SP.

Basically the syscall copies the 4 (CALL,PUSH,PUSH,PUSH) words from the
stack into a local struct (args), adjusts SP, and adjusts IP to point to
the CALL instruction that got us here (retaddr-5).

This way, we get the same context as that #BP would've gotten. Then we
run uprobe crap, and on return:

 - sp changed, we take the (slow) IRET path out, and can just jump
   wherever -- typically right after the CALL that got us here, no need
   to re-adjust the stack and take the trampoline tail.

 - sp didn't change, we take the (fast) sysexit path out, and have to
   re-apply the CALL,PUSH,PUSH,PUSH such that the trampoline tail can
   undo them again.

The added shstk_pop/push() exactly match the above undo/redo of the CALL
(and other stack ops).

> > > I'm questioning if the security posture tweak is worth thinking about for
> > > whatever the level of intersection of uprobes usage and shadow stack is
> > > today.
> > 
> > I have no idea how much code is built with shadow stack enabled today;
> > but I see no point in not supporting uprobes on it. The whole of
> > userspace shadow stacks only ever protects from userspace attacking
> > other userspace -- and that protection isn't changed by this.
> 
> Isn't this just about whether to support an optimization for uprobes?

Yes. But supporting the shstk isn't hard (as per this patch), it exactly
matches what it already does to the normal stack. So I don't see a
reason not to do it.

Anyway, I'm not a huge fan of any of this. I suspect FRED will make all
this fancy code totally irrelevant. But until people have FRED enabled
hardware in large quantities, I suppose this has a use.