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 <[email protected]>
> > ---
> >  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.