netdev - Re: [PATCH bpf-next v2 2/5] selftests/bpf: Add test for __nullable suffix in tp

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]

Message-ID: <CAEf4BzYOa7YDa215petHjuvKBOmuxXwTLwP+hET+7CE=DJG0JQ@mail.gmail.com>
Date: Fri, 6 Sep 2024 14:25:04 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Philo Lu <lulie@...ux.alibaba.com>
Cc: bpf@...r.kernel.org, edumazet@...gle.com, rostedt@...dmis.org, 
	mhiramat@...nel.org, mathieu.desnoyers@...icios.com, martin.lau@...ux.dev, 
	ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, eddyz87@...il.com, 
	song@...nel.org, yonghong.song@...ux.dev, john.fastabend@...il.com, 
	kpsingh@...nel.org, sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, 
	davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, mykolal@...com, 
	shuah@...nel.org, mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com, 
	thinker.li@...il.com, juntong.deng@...look.com, jrife@...gle.com, 
	alan.maguire@...cle.com, davemarchevsky@...com, dxu@...uu.xyz, 
	vmalik@...hat.com, cupertino.miranda@...cle.com, mattbobrowski@...gle.com, 
	xuanzhuo@...ux.alibaba.com, netdev@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 2/5] selftests/bpf: Add test for __nullable
 suffix in tp_btf

On Thu, Sep 5, 2024 at 12:56 AM Philo Lu <lulie@...ux.alibaba.com> wrote:
>
> Add a tracepoint with __nullable suffix in bpf_testmod, and add a
> failure load case:
>
> $./test_progs -t "module_attach"
>  #173/1   module_attach/handle_tp_btf_nullable_bare:OK
>  #173     module_attach:OK
>  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Philo Lu <lulie@...ux.alibaba.com>
> ---
>  .../bpf/bpf_testmod/bpf_testmod-events.h         |  6 ++++++
>  .../selftests/bpf/bpf_testmod/bpf_testmod.c      |  2 ++
>  .../selftests/bpf/prog_tests/module_attach.c     | 14 +++++++++++++-
>  .../bpf/progs/test_module_attach_fail.c          | 16 ++++++++++++++++
>  4 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_module_attach_fail.c
>

[...]

> +
> +static void module_attach_fail(void)
> +{
> +       RUN_TESTS(test_module_attach_fail);
> +}
> +
> +void test_module_attach(void)
> +{
> +       module_attach_succ();
> +       module_attach_fail();
> +}

this is not a good idea to combine existing non-RUN_TESTS test with
RUN_TESTS. The latter is a subtest, while the former is a full test.
Keep them separate, just add a new file for
RUN_TESTS(test_module_attach_fail), or add them to existing
RUN_TESTS(), whatever makes most sense.

> diff --git a/tools/testing/selftests/bpf/progs/test_module_attach_fail.c b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
> new file mode 100644
> index 0000000000000..0f848d8f2f5e8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +#include "bpf_misc.h"
> +
> +SEC("tp_btf/bpf_testmod_test_nullable_bare")
> +__failure __msg("invalid mem access")

would be nice to confirm that register corresponding to nullable_ctx
actually has "_or_null" suffix


> +int BPF_PROG(handle_tp_btf_nullable_bare, struct bpf_testmod_test_read_ctx *nullable_ctx)
> +{
> +       return nullable_ctx->len;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.32.0.3.g01195cf9f
>
>