[PATCH bpf-next 6/8] selftests/bpf: Avoid passing out-of-range values to __retval()

[Pu Lehui]

On 2025/4/30 8:51, Peilin Ye wrote:
> Currently, we pass 0x1234567890abcdef to __retval() for the following
> two tests:
> 
>    verifier_load_acquire/load_acquire_64
>    verifier_store_release/store_release_64
> 
> However, the upper 32 bits of that value are being ignored, since
> __retval() expects an int.  Actually, the tests would still pass even if
> I change '__retval(0x1234567890abcdef)' to e.g. '__retval(0x90abcdef)'.
> 
> Restructure the tests a bit to test the entire 64-bit values properly.
> Do the same to their 8-, 16- and 32-bit variants as well to keep the
> style consistent.
> 
> Fixes: ff3afe5da998 ("selftests/bpf: Add selftests for load-acquire and store-release instructions")
> Signed-off-by: Peilin Ye <yepeilin at google.com>
> ---
>   .../bpf/progs/verifier_load_acquire.c         | 40 +++++++++++++------
>   .../bpf/progs/verifier_store_release.c        | 32 +++++++++++----
>   2 files changed, 52 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
> index 77698d5a19e4..a696ab84bfd6 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
> @@ -10,65 +10,81 @@
>   
>   SEC("socket")
>   __description("load-acquire, 8-bit")
> -__success __success_unpriv __retval(0x12)
> +__success __success_unpriv __retval(0)
>   __naked void load_acquire_8(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x12;"
>   	"*(u8 *)(r10 - 1) = w1;"
> -	".8byte %[load_acquire_insn];" // w0 = load_acquire((u8 *)(r10 - 1));
> +	".8byte %[load_acquire_insn];" // w2 = load_acquire((u8 *)(r10 - 1));
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(load_acquire_insn,
> -		     BPF_ATOMIC_OP(BPF_B, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_10, -1))
> +		     BPF_ATOMIC_OP(BPF_B, BPF_LOAD_ACQ, BPF_REG_2, BPF_REG_10, -1))
>   	: __clobber_all);
>   }
>   
>   SEC("socket")
>   __description("load-acquire, 16-bit")
> -__success __success_unpriv __retval(0x1234)
> +__success __success_unpriv __retval(0)
>   __naked void load_acquire_16(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x1234;"
>   	"*(u16 *)(r10 - 2) = w1;"
> -	".8byte %[load_acquire_insn];" // w0 = load_acquire((u16 *)(r10 - 2));
> +	".8byte %[load_acquire_insn];" // w2 = load_acquire((u16 *)(r10 - 2));
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(load_acquire_insn,
> -		     BPF_ATOMIC_OP(BPF_H, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_10, -2))
> +		     BPF_ATOMIC_OP(BPF_H, BPF_LOAD_ACQ, BPF_REG_2, BPF_REG_10, -2))
>   	: __clobber_all);
>   }
>   
>   SEC("socket")
>   __description("load-acquire, 32-bit")
> -__success __success_unpriv __retval(0x12345678)
> +__success __success_unpriv __retval(0)
>   __naked void load_acquire_32(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x12345678;"
>   	"*(u32 *)(r10 - 4) = w1;"
> -	".8byte %[load_acquire_insn];" // w0 = load_acquire((u32 *)(r10 - 4));
> +	".8byte %[load_acquire_insn];" // w2 = load_acquire((u32 *)(r10 - 4));
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(load_acquire_insn,
> -		     BPF_ATOMIC_OP(BPF_W, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_10, -4))
> +		     BPF_ATOMIC_OP(BPF_W, BPF_LOAD_ACQ, BPF_REG_2, BPF_REG_10, -4))
>   	: __clobber_all);
>   }
>   
>   SEC("socket")
>   __description("load-acquire, 64-bit")
> -__success __success_unpriv __retval(0x1234567890abcdef)
> +__success __success_unpriv __retval(0)
>   __naked void load_acquire_64(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"r1 = 0x1234567890abcdef ll;"
>   	"*(u64 *)(r10 - 8) = r1;"
> -	".8byte %[load_acquire_insn];" // r0 = load_acquire((u64 *)(r10 - 8));
> +	".8byte %[load_acquire_insn];" // r2 = load_acquire((u64 *)(r10 - 8));
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(load_acquire_insn,
> -		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_10, -8))
> +		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_2, BPF_REG_10, -8))
>   	: __clobber_all);
>   }
>   
> diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> index 7e456e2861b4..72f1eb006074 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> @@ -10,13 +10,17 @@
>   
>   SEC("socket")
>   __description("store-release, 8-bit")
> -__success __success_unpriv __retval(0x12)
> +__success __success_unpriv __retval(0)
>   __naked void store_release_8(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x12;"
>   	".8byte %[store_release_insn];" // store_release((u8 *)(r10 - 1), w1);
> -	"w0 = *(u8 *)(r10 - 1);"
> +	"w2 = *(u8 *)(r10 - 1);"
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(store_release_insn,
> @@ -26,13 +30,17 @@ __naked void store_release_8(void)
>   
>   SEC("socket")
>   __description("store-release, 16-bit")
> -__success __success_unpriv __retval(0x1234)
> +__success __success_unpriv __retval(0)
>   __naked void store_release_16(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x1234;"
>   	".8byte %[store_release_insn];" // store_release((u16 *)(r10 - 2), w1);
> -	"w0 = *(u16 *)(r10 - 2);"
> +	"w2 = *(u16 *)(r10 - 2);"
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(store_release_insn,
> @@ -42,13 +50,17 @@ __naked void store_release_16(void)
>   
>   SEC("socket")
>   __description("store-release, 32-bit")
> -__success __success_unpriv __retval(0x12345678)
> +__success __success_unpriv __retval(0)
>   __naked void store_release_32(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"w1 = 0x12345678;"
>   	".8byte %[store_release_insn];" // store_release((u32 *)(r10 - 4), w1);
> -	"w0 = *(u32 *)(r10 - 4);"
> +	"w2 = *(u32 *)(r10 - 4);"
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(store_release_insn,
> @@ -58,13 +70,17 @@ __naked void store_release_32(void)
>   
>   SEC("socket")
>   __description("store-release, 64-bit")
> -__success __success_unpriv __retval(0x1234567890abcdef)
> +__success __success_unpriv __retval(0)
>   __naked void store_release_64(void)
>   {
>   	asm volatile (
> +	"r0 = 0;"
>   	"r1 = 0x1234567890abcdef ll;"
>   	".8byte %[store_release_insn];" // store_release((u64 *)(r10 - 8), r1);
> -	"r0 = *(u64 *)(r10 - 8);"
> +	"r2 = *(u64 *)(r10 - 8);"
> +	"if r2 == r1 goto 1f;"
> +	"r0 = 1;"
> +"1:"
>   	"exit;"
>   	:
>   	: __imm_insn(store_release_insn,
Reviewed-by: Pu Lehui <pulehui at huawei.com>