linux-kernel - Re: [PATCH bpf-next v11 3/3] selftests/bpf: make usdt

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] [day] [month] [year] [list]

Message-ID: <CAEf4BzaGUhYmUyVZbO18fMYaWphPu6btuHomqx-D2Fk_BD__rw@mail.gmail.com>
Date: Wed, 20 Aug 2025 16:25:58 -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 v11 3/3] selftests/bpf: make usdt_o1 reliably
 generate SIB USDT arg spec

On Mon, Aug 18, 2025 at 12:46 AM Jiawei Zhao <phoenix500526@....com> wrote:
>
> usdt_o1 is intended to exercise the SIB (Scale-Index-Base) argument
> handling in libbpf's USDT path. With GCC 13 this reliably produced a
> SIB-form argument (e.g. 8@(%rdx,%rax,8)), but with newer GCC (e.g. 15)
> the compiler frequently optimizes the probe argument into a plain
> register (e.g. 8@...x) or a stack slot, so the test stops covering the
> SIB code path and becomes flaky across toolchains.
>
> Force a SIB memory operand in the probe by:
> * placing the base pointer into %rdx and the index into %rax using an
>   empty inline asm with output constraints ("=d", "=a") and matching
>   inputs
> * immediately passing base[idx] to STAP_PROBE1.
> * only enable on x86 platform.
>
> This makes the compiler encode the operand as SIB (base + index8),
> which in .note.stapsdt shows up as 8@(%rdx,%rax,8) regardless of GCC
> version. A memory clobber and noinline prevent reordering/re-allocation
> around the probe site.
>
> This change is x86_64-specific and does not alter program semantics; it
> only stabilizes the USDT argument shape so the test consistently
> validates SIB handling. Clang historically prefers stack temporaries for
> such operands, but the selftests build with GCC, and this keeps behavior
> stable across GCC versions without introducing a separate .S file.
>
> Signed-off-by: Jiawei Zhao <phoenix500526@....com>
> ---
>  .../testing/selftests/bpf/prog_tests/usdt_o1.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>

See the suggestion on the previous patch. sdt.h has STAP_PROBE_ASM()
macro that allows to trigger USDTs from asm block. I have never used
it, but this looks like a perfect opportunity to make use of it. Can
you please give it a try?

> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt_o1.c b/tools/testing/selftests/bpf/prog_tests/usdt_o1.c
> index 706168e804cb..6c04519b3757 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt_o1.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt_o1.c
> @@ -18,11 +18,19 @@
>  int lets_test_this(int);
>  static volatile __u64 array[1] = {test_value};
>
> -static __always_inline void trigger_func(void)
> +static noinline void trigger_func(void)
>  {
> +#if defined(__x86_64__) || defined(__i386__)
>         /* Base address + offset + (index * scale) */
> -       for (volatile int i = 0; i <= 0; i++)
> -               STAP_PROBE1(test, usdt1, array[i]);
> +       /* Force SIB addressing with inline assembly */
> +       const __u64 *base;
> +       __u32 idx;
> +       /* binding base to %rdx and idx to %rax */
> +       asm volatile("" : "=d"(base), "=a"(idx) : "0"(array), "1"((__u32)0) : "memory");
> +       STAP_PROBE1(test, usdt1, base[idx]);
> +#else
> +       STAP_PROBE1(test, usdt1, array[0]);
> +#endif
>  }
>
>  static void basic_sib_usdt(void)
> @@ -66,5 +74,9 @@ static void basic_sib_usdt(void)
>
>  void test_usdt_o1(void)
>  {
> +#if !defined(__x86_64__) && !defined(__i386__)
> +       test__skip();
> +       return;
> +#endif
>         basic_sib_usdt();
>  }
> --
> 2.43.0
>