Re: [PATCH bpf-next v1 05/18] bpf: Check helper and kfunc mem+size arguments identically

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked nullable pointers passed to helpers with `size > 0` can cause a kernel panic, as the verifier does not enforce a strict correlation between the pointer's runtime NULL state and a zero-size constraint.
--

--- Patch [5]: [PATCH bpf-next v1 05/18] bpf: Check helper and kfunc mem+size arguments identically ---
commit 209f89ec76658f4c319cb2f0b37557c6e39026aa
Author: Amery Hung <ameryhung@gmail.com>

bpf: Check helper and kfunc mem+size arguments identically

This patch unifies the memory and size argument checking between helpers
and kfuncs. By moving the may_be_null handling into check_mem_size_reg(),
it prevents an issue where an un-narrowed map-of-maps value could be
passed to a nullable helper memory argument and leak kernel pointers.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 29d45636c28b8..6520742d51b58 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6873,7 +6873,14 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
>  			      bool zero_size_allowed,
>  			      struct bpf_call_arg_meta *meta)
>  {
> -	int err;
> +	bool may_be_null = type_may_be_null(mem_reg->type);
> +	struct bpf_reg_state saved_reg;
> +	int err = 0;
> +
> +	if (may_be_null) {
> +		saved_reg = *mem_reg;
> +		mark_ptr_not_null_reg(mem_reg);
> +	}

[ ... ]

> -	err = check_helper_mem_access(env, mem_reg, mem_argno, reg_umax(size_reg),
> -				      access_type, zero_size_allowed, meta);
> +
> +	if (access_type & BPF_READ)
> +		err = check_helper_mem_access(env, mem_reg, mem_argno, reg_umax(size_reg),
> +					      BPF_READ, zero_size_allowed, meta);
> +	if (!err && access_type & BPF_WRITE)
> +		err = check_helper_mem_access(env, mem_reg, mem_argno, reg_umax(size_reg),
> +					      BPF_WRITE, zero_size_allowed, meta);
> +
>  	if (!err) {
>  		int regno = reg_from_argno(size_argno);
>  
> @@ -6920,6 +6933,10 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
>  		else
>  			err = mark_stack_arg_precision(env, arg_idx_from_argno(size_argno));
>  	}
> +
> +	if (may_be_null)
> +		*mem_reg = saved_reg;
> +
>  	return err;
>  }

[Severity: High]
This is a pre-existing issue, but by clearing the PTR_MAYBE_NULL flag 
temporarily, does this bypass the verifier's check to ensure the size 
evaluates to 0 when the pointer is NULL at runtime?

If a BPF program obtains a nullable pointer (for instance, from
bpf_map_lookup_elem) and passes it to a helper with a size greater than 0
without a NULL check, the verifier permits this because it verifies the
bounds as if the pointer is valid.

At runtime, if the pointer is NULL, wouldn't the helper execute and 
dereference the NULL pointer? For example, in net/core/filter.c:bpf_csum_diff(),
if from_size or to_size > 0, the pointer is dereferenced by csum_partial():

    if (from_size && to_size)
        ret = csum_sub(csum_partial(to, to_size, ret),
                       csum_partial(from, from_size, 0));
    else if (to_size)
        ret = csum_partial(to, to_size, ret);
    else if (from_size)
        ret = ~csum_partial(from, from_size, ~ret);

Similarly, in kernel/bpf/helpers.c:bpf_snprintf(), bstr_printf() could be
called without checking if str is NULL when str_size > 0:

    err = bstr_printf(str, str_size, fmt, data.bin_args);

Should the verifier emit a runtime check or enforce a constraint to ensure 
the size is 0 if a nullable pointer is passed to prevent a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723050806.1158442-1-ameryhung@gmail.com?part=5