linux-kernel - Re: [PATCH] bpf: fix excessively checking for elem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7d351341-fefe-a40f-f62a-d9505432d056@iogearbox.net>
Date: Fri, 19 Jul 2024 18:22:09 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Lin Feng <linf@...gsu.com>, ast@...nel.org
Cc: bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
 yonghong.song@...ux.dev, Hou Tao <houtao1@...wei.com>,
 Brian Vazquez <brianvv@...gle.com>
Subject: Re: [PATCH] bpf: fix excessively checking for elem_flags in batch
 update mode

On 7/17/24 1:15 PM, Lin Feng wrote:
> Currently generic_map_update_batch will reject all valid command flags for
> BPF_MAP_UPDATE_ELEM other than BPF_F_LOCK, which is overkill, map updating
> semantic does allow specify BPF_NOEXIST or BPF_EXIST even for batching
> update.
> 
> Signed-off-by: Lin Feng <linf@...gsu.com>

[ +Hou/Brian ]

Please also add a BPF selftest along with this extension which exercises the
batch update and validates the behavior for the various flags which are now enabled.

Also, please discuss the semantics in the commit msg.. errors due to BPF_EXIST and
BPF_NOEXIST will cause bpf_map_update_value() to fail and then break the loop. It's
probably fine given batch.count (cp) will be propagated back to user space to tell
how many elements could actually get updated.

> ---
>   kernel/bpf/syscall.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 869265852d51..d85361f9a9b8 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1852,7 +1852,7 @@ int generic_map_update_batch(struct bpf_map *map, struct file *map_file,
>   	void *key, *value;
>   	int err = 0;
>   
> -	if (attr->batch.elem_flags & ~BPF_F_LOCK)
> +	if ((attr->batch.elem_flags & ~BPF_F_LOCK) > BPF_EXIST)
>   		return -EINVAL;
>   
>   	if ((attr->batch.elem_flags & BPF_F_LOCK) &&
>