On Mon, Feb 24, 2025 at 11:22:42AM -0800, Alexei Starovoitov wrote: > On Mon, Feb 24, 2025 at 6:08 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > > > +SYSCALL_DEFINE0(uprobe) > > +{ > > + struct pt_regs *regs = task_pt_regs(current); > > + unsigned long bp_vaddr; > > + int err; > > + > > + err = copy_from_user(&bp_vaddr, (void __user *)regs->sp + 3*8, sizeof(bp_vaddr)); > > + if (err) { > > + force_sig(SIGILL); > > + return -1; > > + } > > + > > + /* Allow execution only from uprobe trampolines. */ > > + if (!in_uprobe_trampoline(regs->ip)) { > > + force_sig(SIGILL); > > + return -1; > > + } > > + > > + handle_syscall_uprobe(regs, bp_vaddr - 5); > > + return 0; > > +} > > + > > +asm ( > > + ".pushsection .rodata\n" > > + ".balign " __stringify(PAGE_SIZE) "\n" > > + "uprobe_trampoline_entry:\n" > > + "endbr64\n" > > why endbr is there? > The trampoline is called with a direct call. ok, that's wrong, will remove that > > > + "push %rcx\n" > > + "push %r11\n" > > + "push %rax\n" > > + "movq $" __stringify(__NR_uprobe) ", %rax\n" > > To avoid introducing a new syscall for a very similar operation > can we disambiguate uprobe vs uretprobe via %rdi or > some other way? > imo not too late to change uretprobe api. > Maybe it was discussed already. yes, I recall discussing that early during uretprobe work with the decision to have separate syscalls for each uprobe and uretprobe.. however wrt recent seccomp changes, it might be easier just to add argument to uretprobe syscall to handle uprobe too bad it's not the other way around.. uprobe syscall with argument to do uretprobe would sound better > > > + "syscall\n" > > + "pop %rax\n" > > + "pop %r11\n" > > + "pop %rcx\n" > > + "ret\n" > > In later patches I see nop5 is replaced with a call to > uprobe_trampoline_entry, but which part saves > rdi and other regs? > Compiler doesn't automatically spill/fill around USDT's nop/nop5. > Selftest is doing: > +__naked noinline void uprobe_test(void) > so just lucky ? if you mean registers that would carry usdt arguments, ebpf programs access those based on assembler operand string stored in usdt record: stapsdt 0x00000048 NT_STAPSDT (SystemTap probe descriptors) Provider: test Name: usdt3 Location: 0x0000000000712f2f, Base: 0x0000000002f516b0, Semaphore: 0x0000000003348ec2 -> Arguments: -4@-1192(%rbp) -8@-1200(%rbp) 8@%rax it's up to bpf program to know which register(+offset) to access, libbpf have all this logic hidden behind usdt_manager_attach_usdt and bpf_usdt_arg bpf call the trampoline only saves rcx/r11/rax, because they are changed by syscall instruction but actually I forgot to load these saved values (of rcx/r11/rax) and rsp into regs that are passed to ebpf program, (like we do in uretprobe syscall) will fix that in next version I'll add tests for optimized usdt with more arguments thanks, jirka