Re: [PATCH bpf-next v3 06/15] bpf: Recognize __nullable on pointer members of kernel structs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: PTR_MAYBE_NULL is lost when reading __nullable members from untrusted pointers
--

commit 6e594b49287fcdc28cc76204149bfee8d35d1dd9
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Recognize __nullable on pointer members of kernel structs

This patch teaches the BPF verifier to recognize the __nullable BTF type tag
on pointer members of kernel structs. It reports these as PTR_MAYBE_NULL,
intended to force BPF programs to check them for NULL before dereferencing
them, even when read through a probed pointer.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f143b1b18b5ca..b4d9ad3ca966b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -6712,9 +6714,9 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
>  			 * aggressively mark as untrusted otherwise such
>  			 * pointers will be plain PTR_TO_BTF_ID without flags
>  			 * and will be allowed to be passed into helpers for
> -			 * compat reasons.
> +			 * compat reasons. A __nullable member stays nullable.
>  			 */
> -			flag = PTR_UNTRUSTED;
> +			flag = PTR_UNTRUSTED | (flag & PTR_MAYBE_NULL);
>  		}

[Severity: Medium]
Does the untrusted pointer path just above this block also need to preserve
PTR_MAYBE_NULL?

Looking at check_ptr_to_btf_access(), if the parent pointer being
dereferenced is untrusted, the code executes this branch:

    } else if (type_flag(reg->type) & PTR_UNTRUSTED) {
        /* If this is an untrusted pointer, all pointers formed by walking it
         * also inherit the untrusted flag.
         */
        flag = PTR_UNTRUSTED;

When btf_struct_access() processes a __nullable pointer member, it sets
PTR_MAYBE_NULL in the flag variable. However, because this assignment
overwrites the flag entirely, the PTR_MAYBE_NULL modifier is discarded.

As a result, the pointer is typed as PTR_TO_BTF_ID | PTR_UNTRUSTED without
PTR_MAYBE_NULL. The verifier forgets that the pointer can be NULL, bypassing
the required NULL check and violating the explicit design intent described
in the commit message.

Will BPF programs be able to bypass the verifier's mandatory NULL check
for __nullable members of untrusted pointers because of this?

>  	} else {
>  		/* Old compat. Deprecated */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015234125.3152346-1-memxor@gmail.com?part=6