Re: [PATCHSET SLOP RFC 1/6] bpf: Support __arena suffix for kfunc arguments

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The verifier skips size and bounds checks for `__arena` kfunc arguments, relying entirely on a 32KB hardware guard region. This allows a malicious BPF program to overshoot the guard region and cause a kernel oops by passing an offset that results in reading or writing a large struct or buffer.
--

--- Patch [1]: [PATCHSET SLOP RFC 1/6] bpf: Support __arena suffix for kfunc arguments ---
commit 228de70b9e4499793706318f5047bb9d51313346
Author: Tejun Heo <[email protected]>

bpf: Support __arena suffix for kfunc arguments
    
This patch adds the __arena suffix for kfunc arguments, allowing BPF
programs to pass arena pointers directly to kfuncs. The verifier rebases
the register at the call site, avoiding the need for open-coded translation
in every kfunc.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6515d4d3c0032..71d1459c8f2a0 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11975,6 +11980,32 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  			continue;
>  		}
>  
> +		if (is_kfunc_arg_arena(btf, &args[i])) {
> +			t = btf_type_skip_modifiers(btf, args[i].type, NULL);
> +			if (verifier_bug_if(!btf_type_is_ptr(t), env,
> +					    "kfunc %s arg#%d has __arena tag on non-pointer",
> +					    func_name, i))
> +				return -EFAULT;

[ ... ]

> +			if (reg->type != PTR_TO_ARENA && reg->type != SCALAR_VALUE) {
> +				verbose(env, "%s is not a pointer to arena or scalar\n",
> +					reg_arg_name(env, argno));
> +				return -EINVAL;
> +			}
> +			cur_aux(env)->arg_arena_regs |= BIT(regno - BPF_REG_1);
> +			continue;
> +		}

[Severity: High]
Does skipping the rest of the checks here bypass BTF memory size and buffer
bounds tracking for __arena arguments?

Because this relies entirely on the hardware guard region to catch out-of-bounds
accesses, could a BPF program pass an offset close to the 4GB arena limit 
(like 4GB - 1) to a kfunc that takes a large struct or buffer size parameter?

The fault handler strictly limits recovery to the 4GB limit plus a 32KB
guard region:

kernel/bpf/arena.c:bpf_arena_handle_page_fault() {
    ...
    if (page_addr < kbase || page_addr >= kbase + SZ_4G + GUARD_SZ / 2)
        return false;
    ...
}

If a kfunc access extends beyond this guard region, it seems like it would
bypass the fault handler and cause a kernel oops or memory corruption.

Should there be size validation for __arena arguments similar to other memory
pointers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1