Messages in this thread Patch in this message |  | | Date | Wed, 5 Aug 2026 23:06:14 +0200 | | From | Sumanth Korikkar <> | | Subject | Re: [PATCH] uprobes: Skip breakpoint installation on non executable vmas |
| |
On Wed, Aug 05, 2026 at 05:14:13PM +0200, Oleg Nesterov wrote: > On 08/05, Sumanth Korikkar wrote: > > > > bpftrace -e 'usdt:./testprogs/usdt_semaphore_test:tracetest:testprobe { > > printf("%s\n", str(arg1) ); exit(); }' > > I am hoping that Andrii and Jiri (cc'ed) can take a look, I know nothing > about usdt... And TBH, I don't even know what RELRO is ;)
Thank you Oleg and Andrii for the feedback.
As far as I understand, relro segment contains the following sections in the usdt_semaphore_test binary (s390) .init_array, .fini_array, .dynamic, .got.
ld.so dynamic linker resolves all the relocations and fills in the got and then calls mprotect() to make the relro region read only. This prevents runtime modification of these data.
> Let me ask a couple of questions for now. > > > expects a semaphore increment of 1, but semaphore gets double incremented > > > > Test program: > > https://github.com/bpftrace/bpftrace/blob/master/tests/testprogs/usdt_semaphore_test.c > > Perhaps you can provide the test-case which I could compile on my > machine without libbpf-usdt/usdt.h? > > And can you explain what the bpftrace cmd above actually does? I mean, > where does it put the uprobe? I guess the ref_ctr_offset argument of > uprobe_register() refers to USDT_DEFINE_SEMA() in that test-case...
Right.
usdt_semaphore_test elf contains the following information in notes section: Displaying notes found in: .note.stapsdt Owner Data size Description stapsdt 0x00000039 NT_STAPSDT (SystemTap probe descriptors) Provider: tracetest Name: testprobe Location: 0x0000000001000742, Base: 0x00000000010007d4, Semaphore: 0x0000000001002024 Arguments: -8@%r1 8@%r2
bpftrace and libbpf reads the uprobe offset and semaphore location from .note.stapsdt and calls bpf_uprobe_multi_link_attach()
bpf_program__attach_usdt() bpf_program__attach_uprobe_multi() bpf_link_create()
kernel side: @[kprobe:__update_ref_ctr, __update_ref_ctr+0 update_ref_ctr+242 uprobe_write+596 uprobe_write_opcode+76 set_swbp+42 install_breakpoint+106 register_for_each_vma+712 uprobe_register+308 bpf_uprobe_multi_link_attach+956 link_create+506 __sys_bpf+678 __s390x_sys_bpf+72 __do_syscall+360 system_call+114 ]
bpftrace -e 'kfunc:uprobe_register { printf("offset=0x%llx ref_ctr_offset=0x%llx\n", args.offset, args.ref_ctr_offset); }'
offset=0x742 ref_ctr_offset=0x1024
> > Reason: .text mapping and RELRO mapping resolve to the same page aligned > > file offset 0 > > 01000000-01001000 r-xp 00000000 5e:01 usdt_semaphore_test (.text) > > 01001000-01002000 r--p 00000000 5e:01 usdt_semaphore_test (RELRO) > > 01002000-01003000 rw-p 00001000 5e:01 usdt_semaphore_test (semaphore) > > > > valid_vma() currently accepts both mappings (which contains executable > > text and RELRO mapping) during uprobe registration, since both have > > VM_MAYEXEC set. This causes register_for_each_vma() to call > > install_breakpoint() twice for the same underlying uprobe offset in the > > process. This means, update_ref_ctr() is called twice for the same > > process, so a usdt semaphore is incremented from 0 to 2. > > So, 2 vmas map the same binary, install_breakpoint() is called twice. > But, the 2nd install_breakpoint() -> ... -> uprobe_write() should see > that the original insn was already replaced by int3, in this case > verify_opcode() returns 0 and uprobe_write() should do nothing. > > And, if this uprobe was optimized before the 2nd install_breakpoint(), > uprobe_write() won't be called. > > Hmm.
I saw the following behaviour (without this patch) for usdt_semaphore_test: sudo bpftrace -e ' kfunc:install_breakpoint { printf("pid=%d comm=%s vaddr=%#lx vm_start=%#lx vm_end=%#lx pgoff=%#lx flags=%#lx exec=%d\n", pid, comm, args.vaddr, args.vma->vm_start, args.vma->vm_end, args.vma->vm_pgoff, args.vma->vm_flags, (args.vma->vm_flags & 0x4) != 0); }' Attached 1 probe pid=3704 comm=bpftrace vaddr=0x1001742 vm_start=0x1001000 vm_end=0x1002000 pgoff=0 flags=0x8100071 exec=0 pid=3704 comm=bpftrace vaddr=0x1000742 vm_start=0x1000000 vm_end=0x1001000 pgoff=0 flags=0x8000075 exec=1
bpftrace -e 'kfunc:__update_ref_ctr { printf("vaddr=%#lx d=%d\n", args.vaddr, args.d); }' Attached 1 probe vaddr=0x1002024 d=1 (increment) vaddr=0x1002024 d=1 (increment) vaddr=0x1002024 d=-1 (decrement) vaddr=0x1002024 d=-1 (decrement)
bpftrace -e ' kretfunc:verify_opcode { printf("verify_opcode vaddr=%#lx ret=%d\n", args.vaddr, retval); }' Attached 1 probe verify_opcode vaddr=0x1001742 ret=1 (install breakpoint) verify_opcode vaddr=0x1000742 ret=1 (install) verify_opcode vaddr=0x1001742 ret=1 (remove) verify_opcode vaddr=0x1000742 ret=1 (remove)
So semphore incremented to 2 when a tracer was attached and decremented back to 0 when tracer was detached. install_breakpoint() was called for both vaddr and succeeded, also remove_breakpoint() succeeded for both vaddr.
> > Installing a breakpoint for mapping without VM_EXEC and > > updating usdt reference counter in that case is not useful. > > > > Skip non VM_EXEC mappings in install_breakpoint(). This fixes semaphore > > double increment as shown in the above usecase. > > > > Signed-off-by: Sumanth Korikkar <sumanthk@linux.ibm.com> > > --- > > kernel/events/uprobes.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 6300b216012c..9e0bbc3cf401 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -1155,6 +1155,9 @@ static int install_breakpoint(struct uprobe *uprobe, struct vm_area_struct *vma, > > bool first_uprobe; > > int ret; > > > > + if (!(vma->vm_flags & VM_EXEC)) > > + return 0; > > + > > Well, but then it makes more sense to change valid_vma() to nack the > non VM_EXEC mappings ?
After looking at your 2012 commit 78a320542e6c ("uprobes: Change valid_vma() to demand VM_MAYEXEC rather than VM_EXEC"), I thought changing it in valid_vma() was not the right approach.
"If a program maps memory as non executable initially, but it has VM_MAYEXEC permission, the program can later call mprotect(PROT_EXEC) to make it executable." So adding VM_EXEC in valid_vma() can be too strict.
Hence, I think install_breakpoint() can be one point where non VM_EXEC mapping can be restricted.
|  |