Re: [PATCH bpf-next v2 07/18] bpf: Check fixed-size mem args of helpers and kfuncs the same way

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] An integer overflow in `check_mem_reg` when calculating the stack access size allows global subprograms to bypass memory bounds checks, leading to out-of-bounds kernel memory accesses.
- [Critical] Global subprograms unconditionally accept literal NULL scalars for non-nullable pointer arguments, potentially leading to a kernel crash due to NULL pointer dereferences.
--

--- Patch [7]: [PATCH bpf-next v2 07/18] bpf: Check fixed-size mem args of helpers and kfuncs the same way ---
commit 7f7a12884bb4c90a70c0a9e694af984eca478eee
Author: Amery Hung <ameryhung@gmail.com>

bpf: Check fixed-size mem args of helpers and kfuncs the same way

Fixed-size memory arguments went through two paths: helpers called
check_helper_mem_access() directly, while kfuncs and global subprogs
used check_mem_reg(). Route the helper MEM_FIXED_SIZE case through
check_mem_reg() too so all three share the same check.

This also fixes a bug in the helper path. When passing a NULL to
PTR_MAYBE_NULL | ARG_PTR_TO_FIXED_SIZE_MEM argument, the program would
be falsely rejected by check_helper_mem_access().

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2e56f726c12a8..4606294e444a8 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6965,10 +6968,12 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg
>  		mark_ptr_not_null_reg(reg);
>  	}
>  
> -	int size = base_type(reg->type) == PTR_TO_STACK ? -(int)mem_size : mem_size;
> +	size = (!helper && base_type(reg->type) == PTR_TO_STACK) ? -(int)mem_size : mem_size;

[Severity: Critical]
This is a pre-existing issue, but does the -(int)mem_size cast introduce an
integer overflow risk for global subprograms?

If a user defines a BTF struct with a size close to U32_MAX (e.g. 0xFFFFFFFC),
-(int)mem_size evaluates to a small positive integer like 4. 

The verifier would only validate this small 4-byte region on the caller's
stack, while the global subprogram is verified separately with a PTR_TO_MEM
of the original massive size.

Could this allow the subprogram to read and write out-of-bounds kernel memory
beyond the provided stack object?

>  
> -	err = check_helper_mem_access(env, reg, argno, size, BPF_READ, true, meta);
> -	err = err ?: check_helper_mem_access(env, reg, argno, size, BPF_WRITE, true, meta);
> +	if (access_type & BPF_READ)
> +		err = check_helper_mem_access(env, reg, argno, size, BPF_READ, true, meta);
> +	if (!err && (access_type & BPF_WRITE))
> +		err = check_helper_mem_access(env, reg, argno, size, BPF_WRITE, true, meta);

[ ... ]

> @@ -9251,7 +9255,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>  			ret = check_func_arg_reg_off(env, reg, argno, ARG_DONTCARE);
>  			if (ret < 0)
>  				return ret;
> -			if (check_mem_reg(env, reg, argno, arg->mem_size, NULL))
> +			if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL))
>  				return -EINVAL;
>  			if (!(arg->arg_type & PTR_MAYBE_NULL) && (reg->type & PTR_MAYBE_NULL)) {

[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for global
subprograms to bypass nullability requirements here?

When check_mem_reg() is called just above this line, it returns 0 early for
literal NULL scalars:

kernel/bpf/verifier.c:check_mem_reg() {
    if (bpf_register_is_null(reg))
        return 0;
    ...
}

Because of this early return, the nullability check immediately following
check_mem_reg() in btf_check_func_arg_match() fails to catch the NULL.
The check relies on the PTR_MAYBE_NULL flag, which a literal NULL scalar
(SCALAR_VALUE) does not have.

Does this allow global subprograms to accept unvalidated NULL pointers
even for arguments not marked as nullable, leading to potential NULL
pointer dereferences?

>  				bpf_log(log, "%s is expected to be non-NULL\n",
>  					reg_arg_name(env, argno));

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