Re: [PATCH bpf v4 1/3] bpf: Fix incorrect handling of user flags in bpf_percpu_array_update

[Leon Hwang <leon.hwang@linux.dev>]

Hi Masoud,

Please wait some time, say 24 hours, for human reviews before sending
the next revision.

On 4/10/26 19:10, Masoud Aghasi wrote:
> For BPF_MAP_TYPE_PERCPU_ARRAY map, bpf_percpu_array_update()
> is not considering the possibility of a combination of
> (BPF_NOEXIST, BPF_EXIST) flags with (BPF_F_CPU, BPF_F_ALL_CPUS) flags.
> This causes the (BPF_NOEXIST, BPF_EXIST) flags to lose their effect
> in some cases.

[...]

> @@ -438,7 +438,8 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
>  	u32 size;
>  	int cpu, off = 0;
>  
> -	if (unlikely((map_flags & BPF_F_LOCK) || (u32)map_flags > BPF_F_ALL_CPUS))
> +	if (unlikely((map_flags & BPF_EXIST) && (map_flags & BPF_NOEXIST)) ||
> +	    unlikely((u32)map_flags & ~(BPF_EXIST | BPF_NOEXIST | BPF_F_CPU | BPF_F_ALL_CPUS)))

Probably, we can add these two macros in include/linux/bpf.h.

#define BPF_EXIST_FLAGS (BPF_EXIST | BPF_NOEXIST)
#define BPF_CPU_FLAGS (BPF_F_CPU | BPF_F_ALL_CPUS)

Then, this 'if' can be simplified to

if (unlikely(((map_flags & BPF_EXIST_FLAGS) == BPF_EXIST_FLAGS) ||
	     ((u32)map_flags & ~(BPF_EXIST_FLAGS | BPF_CPU_FLAGS)))

Also simplify htab_map_check_update_flags() in the next patch by the
same way.

Thanks,
Leon

>  		/* unknown flags */
>  		return -EINVAL;
>  
> @@ -446,7 +447,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
>  		/* all elements were pre-allocated, cannot insert a new one */
>  		return -E2BIG;
>  
> -	if (unlikely(map_flags == BPF_NOEXIST))
> +	if (unlikely(map_flags & BPF_NOEXIST))
>  		/* all elements already exist */
>  		return -EEXIST;
>