Re: [PATCH bpf-next v5 15/16] selftests/bpf: Add negative test for greater-than-8-byte kfunc stack argument

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] The `prog_test_big_arg` struct uses `long`, which fails to test the >8 byte limit on 32-bit architectures and causes BTF size mismatches.
- [Medium] Using `ASSERT_ERR(stack_arg_fail__load(skel))` masks the actual verifier error reason, allowing the negative test to pass for unrelated reasons.
--

commit 1ec644adfded3aa88befcb32fc73958c68b7e018
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Add negative test for greater-than-8-byte kfunc stack argument

This commit adds a selftest to ensure the verifier rejects kfunc calls where
a stack argument exceeds the 8-byte register-sized slot limit. It introduces
a new kfunc taking a struct argument and a BPF program that attempts to pass
a large struct to it.

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> index 2c1cb118f8867..2a40f80b074a3 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> @@ -50,6 +50,11 @@ struct prog_test_pass2 {
>  	} x;
>  };
>  
> +struct prog_test_big_arg {
> +	long a;
> +	long b;
> +};
> +

Does this structure achieve the intended >8 byte size on 32-bit architectures?

When the test kernel module is compiled on a 32-bit host, long is 32-bit,
making the struct exactly 8 bytes. Since the BPF target always treats long
as a 64-bit integer, this makes the struct 16 bytes in the BPF program.

This size discrepancy between the BPF program's BTF and the host kernel's BTF
might cause the verifier to reject the load for a BTF size mismatch rather
than the intended stack argument limit.

Could these fields use explicitly sized types like __u64 to ensure the struct
is consistently larger than 8 bytes across all architectures?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/stack_arg_fail.c b/tools/testing/selftests/bpf/prog_tests/stack_arg_fail.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/stack_arg_fail.c
> @@ -0,0 +1,24 @@
[ ... ]
> +	if (!skel->rodata->has_stack_arg) {
> +		test__skip();
> +		goto out;
> +	}
> +
> +	ASSERT_ERR(stack_arg_fail__load(skel), "load_should_fail");

Could this generic error assertion mask the actual verifier rejection reason?

If the program fails to load due to an unrelated issue, such as a missing BTF,
a syntax error, or the architecture-dependent size mismatch described above,
the test will incorrectly report success because it only checks that the load
operation failed.

Would it be better to refactor this test to use the standard test_loader
infrastructure (via RUN_TESTS) with __failure and __msg annotations
in the BPF program? That would ensure the rejection is explicitly due to the
argument size limit.

> +
> +out:
> +	stack_arg_fail__destroy(skel);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417034658.2625353-1-yonghong.song@linux.dev?part=15