linux-kernel - Re: [PATCH bpf-next 3/4] libbpf: Add support to parse extra info in usdt note record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzZg3sWvD7TwP-V=qw78TF5O6SEt=qJB05b0yOs-27fkEw@mail.gmail.com>
Date: Mon, 24 Nov 2025 09:29:09 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, 
	John Fastabend <john.fastabend@...il.com>
Subject: Re: [PATCH bpf-next 3/4] libbpf: Add support to parse extra info in
 usdt note record

On Mon, Nov 17, 2025 at 12:36 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding support to parse extra info in usdt note record that
> indicates there's nop,nop5 emitted for probe.
>
> We detect this by checking extra zero byte placed in between
> args zero termination byte and desc data end. Please see [1]
> for more details.
>
> Together with uprobe syscall feature detection we can decide
> if we want to place the probe on top of nop or nop5.
>
> [1] https://github.com/libbpf/usdt
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  tools/lib/bpf/usdt.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index c174b4086673..5730295e69d3 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -241,6 +241,7 @@ struct usdt_note {
>         long loc_addr;
>         long base_addr;
>         long sema_addr;
> +       bool nop_combo;
>  };
>
>  struct usdt_target {
> @@ -262,6 +263,7 @@ struct usdt_manager {
>         bool has_bpf_cookie;
>         bool has_sema_refcnt;
>         bool has_uprobe_multi;
> +       bool has_uprobe_syscall;
>  };
>
>  struct usdt_manager *usdt_manager_new(struct bpf_object *obj)
> @@ -301,6 +303,11 @@ struct usdt_manager *usdt_manager_new(struct bpf_object *obj)
>          * usdt probes.
>          */
>         man->has_uprobe_multi = kernel_supports(obj, FEAT_UPROBE_MULTI_LINK);
> +
> +       /*
> +        * Detect kernel support for uprobe syscall to be used to pick usdt attach point.
> +        */

nit: single line comment

but I find the wording confusing, we don't really use uprobe() syscall
to pick USDT attach point (which is what comment implies in my mind).
Just say that we detect uprobe() syscall support. It's presence means
we can take advantage of faster nop5 uprobe handling. Also, please add
reference commit hash + message, just like for other feature detectors
here.

> +       man->has_uprobe_syscall = kernel_supports(obj, FEAT_UPROBE_SYSCALL);
>         return man;
>  }
>
> @@ -784,6 +791,15 @@ static int collect_usdt_targets(struct usdt_manager *man, Elf *elf, const char *
>                 target = &targets[target_cnt];
>                 memset(target, 0, sizeof(*target));
>
> +               /*
> +                * We have usdt with nop,nop5 instruction and we detected uprobe syscall,
> +                * so we can place the uprobe directly on nop5 (+1) to get it optimized.
> +                */
> +               if (note.nop_combo && man->has_uprobe_syscall) {
> +                       usdt_abs_ip++;
> +                       usdt_rel_ip++;
> +               }

how hard would it be to check nop5 instruction in ELF file to be extra
safe? I'm just not sure if I'm 100% comfortable just trusting that
extra zero byte :)

> +
>                 target->abs_ip = usdt_abs_ip;
>                 target->rel_ip = usdt_rel_ip;
>                 target->sema_off = usdt_sema_off;
> @@ -1144,7 +1160,7 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
>  static int parse_usdt_note(GElf_Nhdr *nhdr, const char *data, size_t name_off, size_t desc_off,
>                            struct usdt_note *note)
>  {
> -       const char *provider, *name, *args;
> +       const char *provider, *name, *args, *end, *extra;
>         long addrs[3];
>         size_t len;
>
> @@ -1182,6 +1198,15 @@ static int parse_usdt_note(GElf_Nhdr *nhdr, const char *data, size_t name_off, s
>         if (args >= data + len) /* missing arguments spec */
>                 return -EINVAL;
>
> +       extra = memchr(args, '\0', data + len - args);
> +       if (!extra) /* non-zero-terminated args */
> +               return -EINVAL;
> +       ++extra;
> +       end = data + len;

end variable just to use it once in the comparison below? Also, how
about just this:

extra++;
if (extra < data + len & *extra == '\0')
    note->nop_combo = true;

?

(why assuming extra is the very last byte, maybe we'll have more
"extensions" in the future :) )


> +
> +       /* check if we have one extra byte and if it's zero */
> +       note->nop_combo = (extra + 1) == end && *extra == 0;
> +
>         note->provider = provider;
>         note->name = name;
>         if (*args == '\0' || *args == ':')
> --
> 2.51.1
>