Re: [PATCH bpf-next v1 03/18] bpf: Split kfunc map argument into __const_map and __map

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The newly introduced `KF_ARG_CONST_MAP_PTR` logic fails to verify that the register's base type is `CONST_PTR_TO_MAP`. Any register with a non-zero union aliasing `map_ptr` (such as `PTR_TO_MAP_VALUE`) can bypass validation, leading to severe type confusion at runtime where user-controlled map value memory is treated as a `struct bpf_map *`.
--

--- Patch [3]: [PATCH bpf-next v1 03/18] bpf: Split kfunc map argument into __const_map and __map ---
commit 2aa2cd575b3d4be283e92e30e814910f54fbe0cc
Author: Amery Hung <ameryhung@gmail.com>

bpf: Split kfunc map argument into __const_map and __map

This commit splits the kfunc map argument annotation to avoid type confusion.
__const_map indicates a verifier-known map, while __map indicates an opaque
struct bpf_map resolved at runtime.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index bff51aff25579..0ba0de8eb8fe0 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -12164,6 +12168,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  				}
>  			}
>  			fallthrough;
> +		case KF_ARG_CONST_MAP_PTR:
>  		case KF_ARG_PTR_TO_ITER:
>  		case KF_ARG_PTR_TO_LIST_HEAD:
>  		case KF_ARG_PTR_TO_LIST_NODE:

[Severity: Critical]
Does this skip the is_trusted_reg() checks and leave arg_type as ARG_DONTCARE?
Since KF_ARG_CONST_MAP_PTR falls through to the break below, it seems we might
bypass pointer type safety enforcement here.

[ ... ]
> @@ -12371,12 +12376,16 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
>  			if (ret < 0)
>  				return ret;
>  			break;
> -		case KF_ARG_PTR_TO_MAP:
> -			/* If argument has '__map' suffix expect 'struct bpf_map *' */
> -			ref_id = *reg2btf_ids[CONST_PTR_TO_MAP];
> -			ref_t = btf_type_by_id(btf_vmlinux, ref_id);
> -			ref_tname = btf_name_by_offset(btf, ref_t->name_off);
> -			fallthrough;
> +		case KF_ARG_CONST_MAP_PTR:
> +			if (!reg->map_ptr) {
> +				verbose(env, "pointer in %s isn't map pointer\n",
> +					reg_arg_name(env, argno));
> +				return -EINVAL;
> +			}
> +			ret = process_map_ptr_arg(env, reg, argno, meta);
> +			if (ret < 0)
> +				return ret;
> +			break;

[Severity: Critical]
Is there a missing check for base_type(reg->type) == CONST_PTR_TO_MAP here?

Since this breaks instead of falling through to KF_ARG_PTR_TO_BTF_ID like the
old code did, it looks like any register with a non-zero union aliasing
map_ptr (such as PTR_TO_MAP_VALUE) can bypass validation.

For example, could a BPF program call bpf_wq_init(wq, map__const_map, flags)
and pass a PTR_TO_MAP_VALUE for both the wq and map__const_map arguments?

If wq and map__const_map use the same register, process_map_ptr_arg() would
verify meta->map.ptr == reg->map_ptr (which evaluates to true), and then break
out without ever verifying the register type. This would cause the kernel to
treat user-controlled bytes inside a map value as a kernel struct bpf_map
object, potentially leading to memory corruption.

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