Re: [PATCH bpf v1 1/2] bpf: Reject non-scalar bpf_loop iteration counts

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Sat Sep 5, 2026 at 3:57 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing negative bounds checks for enum array indices in memcg BPF kfuncs (bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events).
> --
>
> commit 3f8d69cede626ae022617ccb30f54b8f41f90edd
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> bpf: Reject non-scalar bpf_loop iteration counts
>
> This commit fixes a verifier warning and subsequent panic triggered when
> pointer values are passed to the bpf_loop() nr_loops argument. It
> introduces ARG_SCALAR to ensure generic helper argument validation
> rejects pointers before loop inlining and precision processing.
>
> Link: https://lore.kernel.org/bpf/6a9ad24c.b5d4176b.238c3e.0001.GAE@google.com/
>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 3a7eb2185c354..591dfab3e706c 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -894,6 +894,7 @@ enum bpf_arg_type {
>>
>>  	ARG_PTR_TO_CTX,		/* pointer to context */
>>  	ARG_ANYTHING,		/* any (initialized) argument is ok */
>> +	ARG_SCALAR,		/* scalar argument */
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing the addition
> of ARG_SCALAR and how the verifier handles scalar arguments, a pre-existing
> missing bounds check was noticed in the memory cgroup kfuncs.
>
> Could negative enum values passed by untrusted BPF programs cause an
> out-of-bounds read in the memcg BPF kfuncs?
>
> Looking at mm/bpf_memcontrol.c:bpf_mem_cgroup_memory_events():
>
> 	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))
> 		return (unsigned long)-1;
>
> 	return atomic_long_read(&memcg->memory_events[event]);
>
> And similarly in mm/bpf_memcontrol.c:bpf_mem_cgroup_vm_events(), which
> exposes validation logic from mm/memcontrol.c:memcg_vm_event_item_valid():
>
> 	if (idx >= NR_VM_EVENT_ITEMS)
> 		return false;
>
> 	return !BAD_STAT_IDX(memcg_events_index(idx));
>
> Because the BPF verifier enforces scalar types but does not validate enum
> ranges, a BPF program can pass a negative integer as the event parameter.
> The bounds checks use signed comparisons without a lower bound (< 0) check,
> so negative values will bypass the validation (e.g., -1 >= 4 evaluates to
> false).
>

My understanding is that since enum values are non-neggative, the enum type is
treated unsigned by default. But I think it might make sense to just cast
explicitly since Sashiko keeps repeating this concern and it is probably clearer
for a reader (I forgot enum values would be unsigned when all enum constants are
non-negative myself).

> Can this lead to an out-of-bounds array access on kernel memory when the
> negative value is used as an index into memcg->memory_events?