linux-kernel - Re: [PATCH bpf-next v3 1/2] bpftool: Use appropriate permissions for map access

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: <544524c2-e913-4181-b43a-07b029ff75e4@kernel.org>
Date: Mon, 16 Jun 2025 15:06:04 +0100
From: Quentin Monnet <qmo@...nel.org>
To: Slava Imameev <slava.imameev@...wdstrike.com>, ast@...nel.org,
 daniel@...earbox.net, andrii@...nel.org, shuah@...nel.org,
 bpf@...r.kernel.org
Cc: martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org,
 yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
 sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, mykolal@...com,
 justin.deschamp@...wdstrike.com, mark.fontana@...wdstrike.com,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 1/2] bpftool: Use appropriate permissions for
 map access

2025-06-12 08:18 UTC+1000 ~ Slava Imameev <slava.imameev@...wdstrike.com>
> Modify several functions in tools/bpf/bpftool/common.c to allow
> specification of requested access for file descriptors, such as
> read-only access.
> 
> Update bpftool to request only read access for maps when write
> access is not required. This fixes errors when reading from maps
> that are protected from modification via security_bpf_map.
> 
> Signed-off-by: Slava Imameev <slava.imameev@...wdstrike.com>
> ---
> Changes in v2:
> - fix for a test compilation error: "conflicting types for 'bpf_fentry_test1'"
> Changes in v3:
> - Addressed review feedback
> - Converted the check for flags to an assert in map_parse_fds
> - Modified map_fd_by_name to keep an existing fd where possible
> - Fixed requested access for map delete command in do_delete
> - Changed requested access to RDONLY for inner map fd in do_create
> - Changed requested access to RDONLY for iterator fd in do_pin
> ---
> ---
>  tools/bpf/bpftool/btf.c           |  3 +-
>  tools/bpf/bpftool/common.c        | 58 ++++++++++++++++++++++---------
>  tools/bpf/bpftool/iter.c          |  2 +-
>  tools/bpf/bpftool/link.c          |  2 +-
>  tools/bpf/bpftool/main.h          | 13 ++++---
>  tools/bpf/bpftool/map.c           | 56 +++++++++++++++++------------
>  tools/bpf/bpftool/map_perf_ring.c |  3 +-
>  tools/bpf/bpftool/prog.c          |  4 +--
>  8 files changed, 91 insertions(+), 50 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 6b14cbfa58aa..1ba27cb03348 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -905,7 +905,8 @@ static int do_dump(int argc, char **argv)
>  			return -1;
>  		}
>  
> -		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> +		fd = map_parse_fd_and_info(&argc, &argv, &info, &len,
> +					   BPF_F_RDONLY);
>  		if (fd < 0)
>  			return -1;
>  
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index ecfa790adc13..3bdc65112c0d 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c

[...]

> @@ -1023,8 +1042,13 @@ static int map_fd_by_name(char *name, int **fds)
>  	return -1;
>  }
>  
> -int map_parse_fds(int *argc, char ***argv, int **fds)
> +int map_parse_fds(int *argc, char ***argv, int **fds, __u32 open_flags)
>  {
> +	LIBBPF_OPTS(bpf_get_fd_by_id_opts, opts);
> +
> +	assert((open_flags & ~BPF_F_RDONLY) == 0);


Can you please "#include <assert.h>" at the top of the file? We don't
need it in the kernel repo, because the header is included from
tools/include/linux/kernel.h (from bpftool's main.h) if I remember
correctly. But the GitHub mirror uses a stripped-down version of
kernel.h which doesn't pull assert.h, so we need to include it
explicitly - I just remembered when seeing your v3, sorry.

Looks good to me otherwise, thanks!
Quentin