Re: [PATCH bpf-next 2/2] selftests/bpf: Cover scalar addition from rdonly untrusted memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The newly added selftest only verifies the rejection of `BPF_ADD` with untrusted pointers, missing coverage for other vulnerable ALU operations.

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the verifier incorrectly allows scalar operations (like BPF_SUB, BPF_AND, BPF_XOR) with untrusted pointers, leading to arbitrary kernel memory read/write vulnerabilities.
--

commit f35b72689eaa4c1dd4c68b268d4611d79f672994
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

selftests/bpf: Cover scalar addition from rdonly untrusted memory

This commit adds a BPF verifier selftest for scalar addition with
rdonly_untrusted_mem. It checks if the verifier properly rejects adding an
untrusted memory value to a scalar destination.

> diff --git a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
> index 5b4453747c230..303b8ed3e70b6 100644
> --- a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
> +++ b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
[ ... ]
> @@ -77,6 +77,23 @@ int offset_not_tracked(void *ctx)
>  	return s;
>  }
>  
> +SEC("socket")
> +__failure
> +__msg("R1 tried to add from rdonly_untrusted_mem to scalar")
> +__naked void scalar_add_not_ok(void)
> +{
> +	asm volatile ("r1 = 0;"
> +	"r2 = 0;"
> +	"call %[bpf_rdonly_cast];"
> +	"r1 = 0;"
> +	"r1 += r0;"

[Severity: Critical]
This is a pre-existing issue, but the verifier incorrectly allows scalar
operations like subtraction, bitwise AND, and XOR with untrusted pointers,
leading to arbitrary kernel memory read/write vulnerabilities.

In kernel/bpf/verifier.c:adjust_reg_min_max_vals(), the check explicitly
tests for addition:

	if (opcode == BPF_ADD &&
	    base_type(src_reg->type) == PTR_TO_MEM &&
	    (src_reg->type & PTR_UNTRUSTED)) {
		verbose(env, "R%d tried to add from %s to scalar\n", ...);

For other ALU operations, the verifier delegates to
adjust_ptr_min_max_vals(), which returns early:

	if (base_type(ptr_reg->type) == PTR_TO_MEM &&
	    (ptr_reg->type & PTR_UNTRUSTED))
		return 0;

This early return bypasses all opcode validation and leaves the destination
register unmodified as a scalar value with its original bounds. At runtime,
the register receives the result of the pointer arithmetic and can be used
to bypass bounds checks.

Could the verifier be updated to properly reject all invalid ALU operations
on untrusted pointers?

[Severity: Low]
Is it possible to expand this selftest to cover other ALU operations?

Because the verifier code currently ignores operations other than addition,
adding tests like scalar_sub_not_ok or scalar_xor_not_ok would ensure these
operations are also correctly rejected and expose the missing verifier checks.

> +	"r0 = 0;"
> +	"exit;"
> +	:
> +	: __imm(bpf_rdonly_cast)
> +	: __clobber_all);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-f01-03-scalar-add-bpf-next-v1-0-e6212e274155@mails.tsinghua.edu.cn?part=2