| 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
| ||
|
Message-ID: <CAEf4Bzb0y+HM1-VG-vYEckeE=+gcc2=4TWdW_7hngKxGiHtwNQ@mail.gmail.com>
Date: Wed, 27 Aug 2025 15:50:39 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiawei Zhao <phoenix500526@....com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
yonghong.song@...ux.dev, bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v16 1/2] libbpf: fix USDT SIB argument handling
causing unrecognized register error
On Tue, Aug 26, 2025 at 10:31 PM Jiawei Zhao <phoenix500526@....com> wrote:
>
> On x86-64, USDT arguments can be specified using Scale-Index-Base (SIB)
> addressing, e.g. "1@-96(%rbp,%rax,8)". The current USDT implementation
> in libbpf cannot parse this format, causing `bpf_program__attach_usdt()`
> to fail with -ENOENT (unrecognized register).
>
> This patch fixes this by implementing the necessary changes:
> - add correct handling for SIB-addressed arguments in `bpf_usdt_arg`.
> - add adaptive support to `__bpf_usdt_arg_type` and
> `__bpf_usdt_arg_spec` to represent SIB addressing parameters.
>
> Signed-off-by: Jiawei Zhao <phoenix500526@....com>
> ---
> tools/lib/bpf/usdt.bpf.h | 44 +++++++++++++++++++++++--
> tools/lib/bpf/usdt.c | 69 +++++++++++++++++++++++++++++++++++++---
> 2 files changed, 106 insertions(+), 7 deletions(-)
>
[...]
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 3373b9d45ac4..e5eeac0b0fa4 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -200,12 +200,23 @@ enum usdt_arg_type {
> USDT_ARG_CONST,
> USDT_ARG_REG,
> USDT_ARG_REG_DEREF,
> + USDT_ARG_SIB,
> };
>
> /* should match exactly struct __bpf_usdt_arg_spec from usdt.bpf.h */
> struct usdt_arg_spec {
> __u64 val_off;
> - enum usdt_arg_type arg_type;
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> + enum usdt_arg_type arg_type: 8;
> + __u16 idx_reg_off: 12;
> + __u16 scale_bitshift: 4;
> + __u8 __reserved: 8; /* keep reg_off offset stable */
> +#else
> + __u8 __reserved: 8; /* keep reg_off offset stable */
> + __u16 idx_reg_off: 12;
> + __u16 scale_bitshift: 4;
> + enum usdt_arg_type arg_type: 8;
> +#endif
> short reg_off;
> bool arg_signed;
> char arg_bitshift;
> @@ -1283,11 +1294,58 @@ static int calc_pt_regs_off(const char *reg_name)
>
> static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg, int *arg_sz)
> {
> - char reg_name[16];
> - int len, reg_off;
> - long off;
> + char reg_name[16] = {0}, idx_reg_name[16] = {0};
> + int len, reg_off, idx_reg_off, scale = 1;
> + long off = 0;
> +
> + if (sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^,] , %d ) %n",
> + arg_sz, &off, reg_name, idx_reg_name, &scale, &len) == 5 ||
> + sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^,] , %d ) %n",
> + arg_sz, reg_name, idx_reg_name, &scale, &len) == 4 ||
> + sscanf(arg_str, " %d @ %ld ( %%%15[^,] , %%%15[^)] ) %n",
> + arg_sz, &off, reg_name, idx_reg_name, &len) == 4 ||
> + sscanf(arg_str, " %d @ ( %%%15[^,] , %%%15[^)] ) %n",
> + arg_sz, reg_name, idx_reg_name, &len) == 3
formatting here was messed up, I fixed it up
> + ) {
> + /*
> + * Scale Index Base case:
> + * 1@-96(%rbp,%rax,8)
> + * 1@(%rbp,%rax,8)
> + * 1@-96(%rbp,%rax)
> + * 1@(%rbp,%rax)
> + */
> + arg->arg_type = USDT_ARG_SIB;
> + arg->val_off = off;
> +
> + reg_off = calc_pt_regs_off(reg_name);
> + if (reg_off < 0)
> + return reg_off;
> + arg->reg_off = reg_off;
>
> - if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n", arg_sz, &off, reg_name, &len) == 3) {
> + idx_reg_off = calc_pt_regs_off(idx_reg_name);
> + if (idx_reg_off < 0)
> + return idx_reg_off;
> + /* validate scale factor and set fields directly */
> + switch (scale) {
> + case 1:
> + arg->scale_bitshift = 0;
> + break;
> + case 2:
> + arg->scale_bitshift = 1;
> + break;
> + case 4:
> + arg->scale_bitshift = 2;
> + break;
> + case 8:
> + arg->scale_bitshift = 3;
> + break;
I made this more compact as well.
BPF selftest looks great, thanks for all the explanations ("d" and "a"
x86-specific register constraints were new to me).
Applied to bpf-next, thanks.
> + default:
> + pr_warn("usdt: invalid SIB scale %d, expected 1, 2, 4, 8\n", scale);
> + return -EINVAL;
> + }
> + arg->idx_reg_off = idx_reg_off;
> + } else if (sscanf(arg_str, " %d @ %ld ( %%%15[^)] ) %n",
> + arg_sz, &off, reg_name, &len) == 3) {
> /* Memory dereference case, e.g., -4@-20(%rbp) */
> arg->arg_type = USDT_ARG_REG_DEREF;
> arg->val_off = off;
> @@ -1306,6 +1364,7 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
> } else if (sscanf(arg_str, " %d @ %%%15s %n", arg_sz, reg_name, &len) == 2) {
> /* Register read case, e.g., -4@...x */
> arg->arg_type = USDT_ARG_REG;
> + /* register read has no memory offset */
> arg->val_off = 0;
>
> reg_off = calc_pt_regs_off(reg_name);
> --
> 2.43.0
>
Powered by blists - more mailing lists