linux-kernel - Re: [PATCH bpf-next] libbpf: parse usdt args without offset on x86 (e.g. 8@(%rsp))

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 2 Dec 2022 16:39:11 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Timo Hunziker <timo.hunziker@....ch>
Cc:     bpf@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
        yhs@...com, john.fastabend@...il.com, kpsingh@...nel.org,
        sdf@...gle.com, haoluo@...gle.com, jolsa@...gle.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next] libbpf: parse usdt args without offset on x86
 (e.g. 8@(%rsp))

On Fri, Dec 2, 2022 at 7:39 AM Timo Hunziker <timo.hunziker@....ch> wrote:
>
> Parse USDT arguments like "8@(%rsp)" on x86. These are emmited by
> systemtap. The syntax is a mixture between the "memory dereference
> case" and the "register read case" as the offset is zero but the
> register is wrapped in parentheses. We treat them the same as the
> the "register read case".

wait, why? I'd assume this is equivalent to 8@0(%rsp) and that's
actually the USDT_ARG_REG_DEREF case? I.e., we read the value of %rsp
and then use that as a pointer to a memory.

>
> I've tested that this fixes the "unrecognized arg #N spec: 8@(%rsp).."
> error I've run into when attaching to a probe with such an argument.
> Attaching and reading the arguments works.
>
> Something similar might be needed for the other supported
> architectures.
>
> ref: https://github.com/libbpf/libbpf/issues/559
>
> Signed-off-by: Timo Hunziker <timo.hunziker@....ch>
> ---
>  tools/lib/bpf/usdt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index b8daae265f99..5e7ec7ad8ad7 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -1233,6 +1233,14 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
>                 if (reg_off < 0)
>                         return reg_off;
>                 arg->reg_off = reg_off;
> +       } else if (sscanf(arg_str, " %d @ ( %%%15[^)] ) %n", &arg_sz, reg_name, &len) == 2) {
> +               /* Register read case with parentheses, e.g., 8@(%rsp) */
> +               arg->arg_type = USDT_ARG_REG;

while you implemented it as "return %rsp value", it's a very different case


> +               arg->val_off = 0;
> +               reg_off = calc_pt_regs_off(reg_name);
> +               if (reg_off < 0)
> +                       return reg_off;
> +               arg->reg_off = reg_off;
>         } 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;
> --
> 2.36.2
>