[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <13a871d1-5bd8-44a1-8737-eb26342d30b4@linux.dev>
Date: Thu, 27 Nov 2025 00:24:10 +0800
From: Leon Hwang <leon.hwang@...ux.dev>
To: bot+bpf-ci@...nel.org, bpf@...r.kernel.org
Cc: ast@...nel.org, andrii@...nel.org, daniel@...earbox.net,
jolsa@...nel.org, yonghong.song@...ux.dev, song@...nel.org,
eddyz87@...il.com, dxu@...uu.xyz, deso@...teo.net, martin.lau@...ux.dev,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, shuah@...nel.org, kerneljasonxing@...il.com,
chen.dylane@...ux.dev, willemb@...gle.com, paul.chaignon@...il.com,
a.s.protopopov@...il.com, memxor@...il.com, yatsenko@...a.com,
tklauser@...tanz.ch, kernel-patches-bot@...com,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
martin.lau@...nel.org, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v12 3/7] bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS
flags support for percpu_hash and lru_percpu_hash maps
On 2025/11/26 23:11, bot+bpf-ci@...nel.org wrote:
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 01a99e3a3..f79d2ae27 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>
> [ ... ]
>
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index c8a9b27f8..441ff5bc5 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>
> [ ... ]
>
>> @@ -1249,6 +1258,15 @@ static long htab_lru_map_update_elem(struct bpf_map *map, void *key, void *value
>> return ret;
>> }
>>
>> +static int htab_map_check_update_flags(bool onallcpus, u64 map_flags)
>> +{
>> + if (unlikely(!onallcpus && map_flags > BPF_EXIST))
>> + return -EINVAL;
>> + if (unlikely(onallcpus && ((map_flags & BPF_F_LOCK) || (u32)map_flags > BPF_F_ALL_CPUS)))
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>
> Can htab_map_check_update_flags() correctly validate the new flags?
>
> When onallcpus=false (the common case for normal map_update_elem
> operations via htab_percpu_map_update_elem()->
> htab_map_update_elem_in_place()), the check at the first if statement
> compares the full 64-bit map_flags value against BPF_EXIST (2).
>
> When BPF_F_CPU (8) is set, or when the CPU number is embedded in the
> upper 32 bits (e.g., 0x0000000100000008 for CPU 1), map_flags will be
> greater than BPF_EXIST, causing this function to return -EINVAL.
>
> This appears inconsistent with the equivalent check in
> bpf_percpu_array_update() (commit 51f40f83b), which uses:
>
> if (unlikely((map_flags & BPF_F_LOCK) || (u32)map_flags > BPF_F_ALL_CPUS))
>
> The percpu_array check casts to u32 to examine only the lower 32 bits,
> allowing the CPU number in the upper 32 bits. The hash table check for
> onallcpus=false doesn't perform this cast, so it will reject valid flag
> combinations that include BPF_F_CPU.
>
> Would this prevent the BPF_F_CPU functionality from working for normal
> percpu_hash update operations?
>
For UAPI percpu_hash updates, this is fine because onallcpus is always true.
For BPF program–initiated percpu_hash updates, the behavior also remains
unchanged since onallcpus is false, and the 'map_flags > BPF_EXIST'
check behaves exactly as it did before.
Thanks,
Leon
>> @@ -1262,9 +1280,9 @@ static long htab_map_update_elem_in_place(struct bpf_map *map, void *key,
>> u32 key_size, hash;
>> int ret;
>>
>> - if (unlikely(map_flags > BPF_EXIST))
>> - /* unknown flags */
>> - return -EINVAL;
>> + ret = htab_map_check_update_flags(onallcpus, map_flags);
>> + if (unlikely(ret))
>> + return ret;
>>
>> WARN_ON_ONCE(!bpf_rcu_lock_held());
>>
>
> [ ... ]
>
>> @@ -1324,9 +1342,9 @@ static long __htab_lru_percpu_map_update_elem(struct bpf_map *map, void *key,
>> u32 key_size, hash;
>> int ret;
>>
>> - if (unlikely(map_flags > BPF_EXIST))
>> - /* unknown flags */
>> - return -EINVAL;
>> + ret = htab_map_check_update_flags(onallcpus, map_flags);
>> + if (unlikely(ret))
>> + return ret;
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19708012130
Powered by blists - more mailing lists