On Wed, Sep 03, 2025 at 11:24:31AM -0700, Andrii Nakryiko wrote:
> On Sun, Jul 20, 2025 at 4:23 AM Jiri Olsa <jo...@kernel.org> wrote:
> >
> > Adding new uprobe syscall that calls uprobe handlers for given
> > 'breakpoint' address.
> >
> > The idea is that the 'breakpoint' address calls the user space
> > trampoline which executes the uprobe syscall.
> >
> > The syscall handler reads the return address of the initial call
> > to retrieve the original 'breakpoint' address. With this address
> > we find the related uprobe object and call its consumers.
> >
> > Adding the arch_uprobe_trampoline_mapping function that provides
> > uprobe trampoline mapping. This mapping is backed with one global
> > page initialized at __init time and shared by the all the mapping
> > instances.
> >
> > We do not allow to execute uprobe syscall if the caller is not
> > from uprobe trampoline mapping.
> >
> > The uprobe syscall ensures the consumer (bpf program) sees registers
> > values in the state before the trampoline was called.
> >
> > Acked-by: Andrii Nakryiko <and...@kernel.org>
> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> > ---
> > arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> > arch/x86/kernel/uprobes.c | 139 +++++++++++++++++++++++++
> > include/linux/syscalls.h | 2 +
> > include/linux/uprobes.h | 1 +
> > kernel/events/uprobes.c | 17 +++
> > kernel/sys_ni.c | 1 +
> > 6 files changed, 161 insertions(+)
> >
> > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
> > b/arch/x86/entry/syscalls/syscall_64.tbl
> > index cfb5ca41e30d..9fd1291e7bdf 100644
> > --- a/arch/x86/entry/syscalls/syscall_64.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> > @@ -345,6 +345,7 @@
> > 333 common io_pgetevents sys_io_pgetevents
> > 334 common rseq sys_rseq
> > 335 common uretprobe sys_uretprobe
> > +336 common uprobe sys_uprobe
> > # don't use numbers 387 through 423, add new calls after the last
> > # 'common' entry
> > 424 common pidfd_send_signal sys_pidfd_send_signal
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > index 6c4dcbdd0c3c..d18e1ae59901 100644
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -752,6 +752,145 @@ void arch_uprobe_clear_state(struct mm_struct *mm)
> > hlist_for_each_entry_safe(tramp, n, &state->head_tramps, node)
> > destroy_uprobe_trampoline(tramp);
> > }
> > +
> > +static bool __in_uprobe_trampoline(unsigned long ip)
> > +{
> > + struct vm_area_struct *vma = vma_lookup(current->mm, ip);
> > +
> > + return vma && vma_is_special_mapping(vma, &tramp_mapping);
> > +}
> > +
> > +static bool in_uprobe_trampoline(unsigned long ip)
> > +{
> > + struct mm_struct *mm = current->mm;
> > + bool found, retry = true;
> > + unsigned int seq;
> > +
> > + rcu_read_lock();
> > + if (mmap_lock_speculate_try_begin(mm, &seq)) {
> > + found = __in_uprobe_trampoline(ip);
> > + retry = mmap_lock_speculate_retry(mm, seq);
> > + }
> > + rcu_read_unlock();
> > +
> > + if (retry) {
> > + mmap_read_lock(mm);
> > + found = __in_uprobe_trampoline(ip);
> > + mmap_read_unlock(mm);
> > + }
> > + return found;
> > +}
> > +
> > +/*
> > + * See uprobe syscall trampoline; the call to the trampoline will push
> > + * the return address on the stack, the trampoline itself then pushes
> > + * cx, r11 and ax.
> > + */
> > +struct uprobe_syscall_args {
> > + unsigned long ax;
> > + unsigned long r11;
> > + unsigned long cx;
> > + unsigned long retaddr;
> > +};
> > +
> > +SYSCALL_DEFINE0(uprobe)
> > +{
> > + struct pt_regs *regs = task_pt_regs(current);
> > + struct uprobe_syscall_args args;
> > + unsigned long ip, sp;
> > + int err;
> > +
> > + /* Allow execution only from uprobe trampolines. */
> > + if (!in_uprobe_trampoline(regs->ip))
> > + goto sigill;
>
> Hey Jiri,
>
> So I've been thinking what's the simplest and most reliable way to
> feature-detect support for this sys_uprobe (e.g., for libbpf to know
> whether we should attach at nop5 vs nop1), and clearly that would be
> to try to call uprobe() syscall not from trampoline, and expect some
> error code.
>
> How bad would it be to change this part to return some unique-enough
> error code (-ENXIO, -EDOM, whatever).
>
> Is there any reason not to do this? Security-wise it will be just fine, right?
good question.. maybe :) the sys_uprobe sigill error path followed the
uprobe logic when things go bad, seem like good idea to be strict
I understand it'd make the detection code simpler, but it could just
just fork and check for sigill, right?
jirka
>
> > +
> > + err = copy_from_user(&args, (void __user *)regs->sp, sizeof(args));
> > + if (err)
> > + goto sigill;
> > +
> > + ip = regs->ip;
> > +
>
> [...]
