[PATCH bpf-next v4 9/9] selftests/bpf: test fsession mixed with fentry and fexit

Menglong Dong posted 9 patches 1 month, 3 weeks ago
[PATCH bpf-next v4 9/9] selftests/bpf: test fsession mixed with fentry and fexit
Posted by Menglong Dong 1 month, 3 weeks ago
Test the fsession when it is used together with fentry, fexit.

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 .../selftests/bpf/progs/fsession_test.c       | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/fsession_test.c b/tools/testing/selftests/bpf/progs/fsession_test.c
index f7c96ef1c7a9..223a6ea47888 100644
--- a/tools/testing/selftests/bpf/progs/fsession_test.c
+++ b/tools/testing/selftests/bpf/progs/fsession_test.c
@@ -190,3 +190,37 @@ int BPF_PROG(test10, int a)
 		test10_exit_ok = *cookie == 0x1111222233334444ull;
 	return 0;
 }
+
+__u64 test11_entry_result = 0;
+__u64 test11_exit_result = 0;
+
+SEC("fsession/bpf_fentry_test1")
+int BPF_PROG(test11, int a, int ret)
+{
+	__u64 *cookie = bpf_fsession_cookie(ctx);
+
+	if (!bpf_fsession_is_return(ctx)) {
+		test11_entry_result = a == 1 && ret == 0;
+		*cookie = 0x123456ULL;
+		return 0;
+	}
+
+	test11_exit_result = a == 1 && ret == 2 && *cookie == 0x123456ULL;
+	return 0;
+}
+
+__u64 test12_result = 0;
+SEC("fexit/bpf_fentry_test1")
+int BPF_PROG(test12, int a, int ret)
+{
+	test12_result = a == 1 && ret == 2;
+	return 0;
+}
+
+__u64 test13_result = 0;
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test13, int a)
+{
+	test13_result = a == 1;
+	return 0;
+}
-- 
2.52.0
Re: [PATCH bpf-next v4 9/9] selftests/bpf: test fsession mixed with fentry and fexit
Posted by bot+bpf-ci@kernel.org 1 month, 3 weeks ago
> diff --git a/tools/testing/selftests/bpf/progs/fsession_test.c b/tools/testing/selftests/bpf/progs/fsession_test.c
> index f7c96ef1c7a9..223a6ea47888 100644
> --- a/tools/testing/selftests/bpf/progs/fsession_test.c
> +++ b/tools/testing/selftests/bpf/progs/fsession_test.c
> @@ -190,3 +190,37 @@ int BPF_PROG(test10, int a)
>  		test10_exit_ok = *cookie == 0x1111222233334444ull;
>  	return 0;
>  }
> +
> +__u64 test11_entry_result = 0;
> +__u64 test11_exit_result = 0;
> +
> +SEC("fsession/bpf_fentry_test1")
> +int BPF_PROG(test11, int a, int ret)
> +{
> +	__u64 *cookie = bpf_fsession_cookie(ctx);
> +
> +	if (!bpf_fsession_is_return(ctx)) {
> +		test11_entry_result = a == 1 && ret == 0;
> +		*cookie = 0x123456ULL;

Should the cookie pointer be checked for NULL before dereferencing?

Looking at the existing tests in this file, both test9 and test10 (which
also call bpf_fsession_cookie) explicitly check if the cookie pointer is
NULL before dereferencing it:

  test9: if (cookie) { *cookie = 0xAAAA...; }
  test10: if (cookie) { *cookie = 0x1111...; }

test11 dereferences cookie without this check. If cookie were NULL or
invalid, test11 would crash rather than fail gracefully like test9/test10
do. While the BPF verifier likely ensures cookie is valid when the
program is loaded, following the established pattern would make the test
more robust.

The same issue occurs in the exit path:

> +		return 0;
> +	}
> +
> +	test11_exit_result = a == 1 && ret == 2 && *cookie == 0x123456ULL;
                                                    ^^^^^^^

> +	return 0;
> +}
> +
> +__u64 test12_result = 0;
> +SEC("fexit/bpf_fentry_test1")
> +int BPF_PROG(test12, int a, int ret)
> +{
> +	test12_result = a == 1 && ret == 2;
> +	return 0;
> +}
> +
> +__u64 test13_result = 0;
> +SEC("fentry/bpf_fentry_test1")
> +int BPF_PROG(test13, int a)
> +{
> +	test13_result = a == 1;
> +	return 0;
> +}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20299185010