Re: [PATCH bpf-next v2 1/2] bpf: reject rdonly/rdwr_buf_size kfunc arguments that exceed u32 max

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] 32-bit integer overflow in `hid_bpf_get_data()` allows arbitrary out-of-bounds kernel memory access.
- [High] Explicit 0-sized requests to kfuncs result in out-of-bounds read due to verifier BTF fallback.
--

--- Patch [1]: [PATCH bpf-next v2 1/2] bpf: reject rdonly/rdwr_buf_size kfunc arguments that exceed u32 max ---
commit 440bc8eb1e0c84aa93160b0ebdb9ef4731dce2ea
Author: Nicholas Dudar <main.kalliope@gmail.com>

bpf: reject rdonly/rdwr_buf_size kfunc arguments that exceed u32 max

This commit fixes a truncation issue where a 64-bit size argument was
incorrectly truncated into a 32-bit field without a bounds check. It rejects
sizes that exceed U32_MAX in check_kfunc_args() before assigning to the
PTR_TO_MEM register.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 233472a871be5..c7fabe50e4874 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12097,6 +12097,11 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  				}
>  
>  				meta->r0_size = reg->var_off.value;

[Severity: High]
This is a pre-existing issue, but how does the verifier handle an explicit
0-sized buffer request here?

If a BPF program calls hid_bpf_get_data() with rdwr_buf_size = 0 and
offset = ctx->allocated_size, the bounds check passes:

drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_get_data()
    if (rdwr_buf_size + offset > ctx->allocated_size)
        return NULL;

The function legally returns a pointer exactly at the end of the buffer.
However, in check_kfunc_call(), the verifier seems to misinterpret an
explicit 0 size as an absent size:

kernel/bpf/verifier.c:check_kfunc_call()
    if (!meta.r0_size) {
        __u32 sz;
        if (!IS_ERR(btf_resolve_size(desc_btf, ptr_type, &sz))) {
            meta.r0_size = sz;
            meta.r0_rdonly = true;
        }
    }

Does this fall back to the BTF type size (1 byte for __u8 *) and incorrectly
grant a 1-byte read permission from the out-of-bounds pointer?

> +				if (meta->r0_size > U32_MAX) {

[Severity: High]
This is a pre-existing issue, but does allowing the size to reach U32_MAX
expose a 32-bit integer overflow in hid_bpf_get_data()?

If a BPF program calls hid_bpf_get_data() with rdwr_buf_size = U32_MAX and
offset = 1, on 32-bit architectures size_t is 32 bits. This would cause the
addition to wrap around to 0:

drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_get_data()
    if (rdwr_buf_size + offset > ctx->allocated_size)
        return NULL;

This bypasses the bounds check and returns a pointer to data + 1. Since the
verifier records the returned register's mem_size as U32_MAX, does this
allow arbitrary out-of-bounds read/write access to kernel memory?

> +					verbose(env, "%s rdonly/rdwr_buf_size exceeds u32 max\n",
> +						reg_arg_name(env, argno));
> +					return -EINVAL;
> +				}
>  				if (regno >= 0)
>  					ret = mark_chain_precision(env, regno);
>  				else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709155837.1879230-1-main.kalliope@gmail.com?part=1