Re: [PATCH bpf 7/8] bpf: mark the zero register precise for a register-form NULL check

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The verifier fails to mark scalar registers precise when `bpf_register_is_null()` is used to satisfy a nullable pointer argument or kptr store.
--

commit c721af04f2965a6636b3260c058c2531da5526e0
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: mark the zero register precise for a register-form NULL check

This patch fixes a bug in check_cond_jmp_op() where comparing a nullable
pointer to a scalar known to be zero fails to mark the scalar register
precise. This prevents malicious state pruning where an imprecise scalar
could bypass safety checks and lead to a forged pointer dereference.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 303368460ec10..fde5d046b6e32 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17052,6 +17052,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
>  	    type_may_be_null(dst_reg->type) &&
>  	    ((BPF_SRC(insn->code) == BPF_K && insn->imm == 0) ||
>  	     (BPF_SRC(insn->code) == BPF_X && bpf_register_is_null(src_reg)))) {
> +		/*
> +		 * For BPF_X the zero is a property of this execution path,
> +		 * hence src_reg has to be precise.
> +		 */
> +		if (BPF_SRC(insn->code) == BPF_X) {
> +			err = mark_chain_precision(env, insn->src_reg);
> +			if (err)
> +				return err;
> +		}

[Severity: Critical]
This is a pre-existing issue and not a bug introduced by this patch, but
since this change addresses missing precision tracking for zero-valued
scalar registers, do we need similar mark_chain_precision() calls in
other places that use bpf_register_is_null() to satisfy pointer checks?

For example, in check_map_kptr_access(), we bypass type checking when the
register is a scalar zero:

kernel/bpf/verifier.c:check_map_kptr_access() {
    ...
    if (!bpf_register_is_null(val_reg) &&
        map_kptr_match_type(env, kptr_field, val_reg, value_regno))
    ...
}

Similar bypasses without precision marking appear in several other argument
checkers:

kernel/bpf/verifier.c:check_func_arg() {
    ...
    if (bpf_register_is_null(reg) && type_may_be_null(arg_type))
        /* A NULL register has a SCALAR_VALUE type, so skip
         * type checking.
         */
        goto skip_type_check;
    ...
    if (type_may_be_null(arg_type) && bpf_register_is_null(reg))
        return 0;
    ...
}

kernel/bpf/verifier.c:check_kfunc_args() {
    ...
    if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type))
        continue;
    ...
}

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

Could an attacker use these paths in a similar way to the vulnerability
described in your commit message? 

If a branch assigns a scalar 0, passes it to one of these functions
(leaving it imprecise), and another branch merges with an arbitrary kernel
address, wouldn't states_equal() incorrectly prune the malicious branch
because regsafe() returns true for imprecise scalars?

>  		/* Mark all identical registers in each branch as either
>  		 * safe or unknown depending R == 0 or R != 0 conditional.
>  		 */

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