[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <f33b87ffc7d3884dab5f73d90ba3f29afd4ee5975be512532702228363cde72e@mail.kernel.org>
Date: Wed, 26 Nov 2025 15:11:36 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: leon.hwang@...ux.dev,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,leon.hwang@...ux.dev,kernel-patches-bot@...com,linux-kernel@...r.kernel.org,linux-kselftest@...r.kernel.org,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,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
> 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?
> @@ -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