[RFC][PATCH v3 00/15] bpf: Per-operation map permissions

[RFC][PATCH v3 00/15] bpf: Per-operation map permissions

[Roberto Sassu]

With the bpf_map security hook, an eBPF program is able to restrict access
to a map. For example, it might allow only read accesses and deny write
accesses.

Unfortunately, libbpf and bpftool don't specify permissions accurately.
Regardless of the operation, they always request read-write permissions. As
a consequence, even if they request a read-like operation, such as a map
lookup, that operation unexpectedly fails even if the eBPF program allows
it.

Even worse, since search operations also request read-write permissions,
the iteration stops when a write-protected map is encountered, making the
other maps inaccessible even if operations on them are allowed.

At low level, the problem is that libbpf and bpftool always set open_flags
and file_flags to zero, when they request a file descriptor for a map.
Unfortunately, the kernel translates this into a request with read-write
permissions. open_flags and file_flags can be set for example to
BPF_F_RDONLY or BPF_F_WRONLY respectively for read-like and write-like
permissions.

Which permissions should be requested depends on the type of operation
performed. For example, map lookup or show are clearly read-like
operations. This can be also seen in the kernel: map_lookup_elem(), first
gets the permissions from the file descriptor and then checks if the
FMODE_CAN_READ flag is set. If yes, map_lookup_elem() continues its
execution.

The goal of this patch set is to propagate the needed permissions from the
implementation of the bpftool subcommands, where the exact permissions can
be determined, down to libbpf and ultimately to the kernel.

To ensure that permissions can be specified for every bpftool operation,
changes are implemented bottom-up. First, libbpf is extended with an _opts
variant for each bpf_*_get_fd_by_id() function, and for bpf_obj_get(). The
original version of those functions is kept for compatibility reasons.

Second, functions in bpftool that directly called the original version now
call the _opts variant, and are modified by introducing a new opts
parameter to propagate it to the callers. This process is repeated until
reaching the leaf, the implementation of the bpftool subcommand where
finally the correct permissions can be set.

One exception to the propagation is made for map search operations. In this
case, the operation is clearly read-like, so the permission requested is
always read-only, regardless of what the caller passed. If the caller
requested additional permissions, and there is a match in the search, a new
file descriptor with needed permissions is obtained.

The _opts variants also accept NULL as argument. In this case, open_flags
and file_flags will be still set to zero as before this patch set. This
makes it possible to select the operations for which we would like to set
permissions more precisely, and leave the rest as it is.

This patch set has been tested by developing a small eBPF program that
introduces three maps: one with read-only access (data_input), one with
read-write access (data_input_w), and finally one for perf-related tests
(data_input_perf).

The test checks the _opts variants introduced in libbpf, and also the
modifications done to bpftool. The test ensures that operations succeeds
when the correct permissions are specified, and also ensures that the
operations cannot be done when insufficient permissions were requested.
Finally, the test ensures that the search operation works as expected,
that it is not blocked on write-protected maps.

This patch set does not fix the case of read-protected maps
(confidentiality use case). Since search requires the read permission,
iteration will stop as per the current behavior. A way to fix that could
be to introduce a new eBPF command, BPF_OBJ_GET_INFO_BY_ID, which could
call the bpf_map security hook with O_ACCESS open flags. LSMs could then
be aware that this is a request for attributes, and not a read of map data,
and enforce policies with finer granularity.

The patch set is organized as follows.

Patch 1 allows users to build bpftool with static libraries, for increased
portability of the binary, especially for testing.

Patch 2 sets open_flags as last field for bpf_*_get_fd_by_id(), except for
bpf_map_get_fd_by_id() for which that field is already set.

Patches 3-7 introduce the _opts variant for the bpf_*_get_fd_by_id() and
bpf_obj_get() functions.

Patches 8-13 propagate the bpf_get_fd_opts structure from libbpf to the
bpftool subcommand implementation and map search, and complete the switch
to the new libbpf functions.

Patch 14 adjusts permissions for map operations depending on the operation
type.

Finally, patch 15 adds a test to verify the correctness of permissions
requested.

As future work, permissions can be set for other eBPF objects as well, such
as progs and links, as soon as the kernel supports access control on them.

Changelog

v2:
  - Add a new parameter to each bpf_*_get_fd_by_id_opts() function, for
    symmetry (suggested by Andrii)
  - Always call the _opts variant of bpf_*_get_fd_by_id() and bpf_obj_get()
    in bpftool
  - Replace flags parameter with opts (suggested by Andrii)
  - Set open_flags as last field of bpf_prog_by_id(),
    bpf_btf_get_fd_by_id() and bpf_link_get_fd_by_id() in the kernel
  - Add open_flags kernel feature autodetection
  - Directly set the bpftool command line in the test without using
    snprintf() (suggested by Andrii)
  - Request write-only permission for update and delete map operations
  - Request read-only permission for pinning maps, and for retrieving the
    file descriptor of the inner map in map of maps
  - Add patch to build bpftool with static libraries
  - Add more tests: map of maps, perf, iterator

v1:
  - Define per-operation permissions rather than retrying access with
    read-only permission (suggested by Daniel)
    https://lore.kernel.org/bpf/20220530084514.10170-1-roberto.sassu@huawei.com/

Roberto Sassu (15):
  bpftool: Attempt to link static libraries
  bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id()
    funcs
  libbpf: Introduce bpf_prog_get_fd_by_id_opts()
  libbpf: Introduce bpf_map_get_fd_by_id_opts()
  libbpf: Introduce bpf_btf_get_fd_by_id_opts()
  libbpf: Introduce bpf_link_get_fd_by_id_opts()
  libbpf: Introduce bpf_obj_get_opts()
  bpftool: Add opts parameter to open_obj_pinned_any() and
    open_obj_pinned()
  bpftool: Add opts parameter to *_parse_fd() functions
  bpftool: Add opts parameter to *_parse_fds()
  bpftool: Add opts parameter to map_parse_fd_and_info()
  bpftool: Add opts parameter in struct_ops functions
  bpftool: Complete switch to bpf_*_get_fd_by_id_opts()
  bpftool: Adjust map permissions
  selftests/bpf: Add map access tests

 kernel/bpf/syscall.c                          |   6 +-
 tools/bpf/bpftool/Makefile                    |  22 +++
 tools/bpf/bpftool/btf.c                       |  27 ++-
 tools/bpf/bpftool/btf_dumper.c                |   2 +-
 tools/bpf/bpftool/cgroup.c                    |   8 +-
 tools/bpf/bpftool/common.c                    |  85 +++++---
 tools/bpf/bpftool/iter.c                      |   6 +-
 tools/bpf/bpftool/link.c                      |  15 +-
 tools/bpf/bpftool/main.h                      |  25 ++-
 tools/bpf/bpftool/map.c                       |  56 ++++--
 tools/bpf/bpftool/map_perf_ring.c             |   7 +-
 tools/bpf/bpftool/net.c                       |   2 +-
 tools/bpf/bpftool/prog.c                      |  22 ++-
 tools/bpf/bpftool/struct_ops.c                |  55 ++++--
 tools/lib/bpf/bpf.c                           |  87 +++++++-
 tools/lib/bpf/bpf.h                           |  19 ++
 tools/lib/bpf/libbpf.c                        |   4 +
 tools/lib/bpf/libbpf.map                      |   5 +
 tools/lib/bpf/libbpf_internal.h               |   3 +
 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../bpf/prog_tests/map_check_access.c         | 186 ++++++++++++++++++
 .../bpf/progs/test_map_check_access.c         | 112 +++++++++++
 22 files changed, 657 insertions(+), 100 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/map_check_access.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_map_check_access.c

-- 
2.25.1

[RFC][PATCH v3 01/15] bpftool: Attempt to link static libraries

[Roberto Sassu]

Introduce the BPFTOOL_STATIC make variable, to let users build bpftool with
static libraries (when set to 1). This increases the portability of the
binary, especially for testing.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/Makefile | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 4b09a5c3b9f1..4628df456245 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -98,6 +98,13 @@ FEATURE_TESTS = libbfd libbfd-liberty libbfd-liberty-z \
 FEATURE_DISPLAY = libbfd libbfd-liberty libbfd-liberty-z \
 		  disassembler-four-args libcap clang-bpf-co-re
 
+ifeq ($(BPFTOOL_STATIC),1)
+FEATURE_CHECK_LDFLAGS-libbfd = -static
+FEATURE_CHECK_LDFLAGS-libbfd-liberty = -static
+FEATURE_CHECK_LDFLAGS-libbfd-liberty-z = -static
+FEATURE_CHECK_LDFLAGS-libcap = -static
+endif
+
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
 ifdef MAKECMDGOALS
@@ -122,8 +129,12 @@ LIBS = $(LIBBPF) -lelf -lz
 LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
 ifeq ($(feature-libcap), 1)
 CFLAGS += -DUSE_LIBCAP
+ifeq ($(BPFTOOL_STATIC),1)
+LIBS += -l:libcap.a
+else
 LIBS += -lcap
 endif
+endif
 
 include $(wildcard $(OUTPUT)*.d)
 
@@ -133,6 +144,16 @@ BFD_SRCS = jit_disasm.c
 
 SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
 
+ifeq ($(BPFTOOL_STATIC),1)
+ifeq ($(feature-libbfd),1)
+  LIBS += -l:libbfd.a -ldl -l:libopcodes.a
+else ifeq ($(feature-libbfd-liberty),1)
+  LIBS += -l:libbfd.a -ldl -l:libopcodes.a -l:libiberty.a
+else ifeq ($(feature-libbfd-liberty-z),1)
+  LIBS += -l:libbfd.a -ldl -l:libopcodes.a -l:libiberty.a -l:libz.a
+  LIBS := $(filter-out -lz, $(LIBS))
+endif
+else
 ifeq ($(feature-libbfd),1)
   LIBS += -lbfd -ldl -lopcodes
 else ifeq ($(feature-libbfd-liberty),1)
@@ -140,6 +161,7 @@ else ifeq ($(feature-libbfd-liberty),1)
 else ifeq ($(feature-libbfd-liberty-z),1)
   LIBS += -lbfd -ldl -lopcodes -liberty -lz
 endif
+endif
 
 ifneq ($(filter -lbfd,$(LIBS)),)
 CFLAGS += -DHAVE_LIBBFD_SUPPORT
-- 
2.25.1

[RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Roberto Sassu]

The bpf() system call validates the bpf_attr structure received as
argument, and considers data until the last field, defined for each
operation. The remaing space must be filled with zeros.

Currently, for bpf_*_get_fd_by_id() functions except bpf_map_get_fd_by_id()
the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space
will result in bpf() rejecting the argument.

Set open_flags as last field for the remaining bpf_*_get_fd_by_id()
functions, so that this information can be taken into account by the bpf()
system call.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 kernel/bpf/syscall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83c7136c5788..b4311155d535 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3689,7 +3689,7 @@ struct bpf_prog *bpf_prog_get_curr_or_next(u32 *id)
 	return prog;
 }
 
-#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD prog_id
+#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD open_flags
 
 struct bpf_prog *bpf_prog_by_id(u32 id)
 {
@@ -4315,7 +4315,7 @@ static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr)
 	return btf_new_fd(attr, uattr);
 }
 
-#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
+#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD open_flags
 
 static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
 {
@@ -4733,7 +4733,7 @@ struct bpf_link *bpf_link_get_curr_or_next(u32 *id)
 	return link;
 }
 
-#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id
+#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD open_flags
 
 static int bpf_link_get_fd_by_id(const union bpf_attr *attr)
 {
-- 
2.25.1

[RFC][PATCH v3 03/15] libbpf: Introduce bpf_prog_get_fd_by_id_opts()

[Roberto Sassu]

Define a new data structure called bpf_get_fd_opts, with the member flags,
to be used by callers of the _opts variants of bpf_*_get_fd_by_id() and
bpf_obj_get() to specify the permissions needed for the operations on the
obtained file descriptor.

Define only one data structure for all variants, as the open_flags field in
bpf_attr for bpf_*_get_fd_by_id_opts() and file_flags for
bpf_obj_get_opts() have the same meaning. Also, it would avoid the
confusion when the same bpftool function calls both
bpf_*_get_fd_by_id_opts() and bpf_obj_get_opts() (e.g. map_parse_fds()).

Then, introduce the new feature FEAT_GET_FD_BY_ID_OPEN_FLAGS to determine
whether or not the kernel supports setting open_flags for
bpf_*_get_fd_by_id() functions (except for bpf_map_get_fd_by_id(), which
already can get it). If the feature is not detected, the _opts variants
ignore flags in the bpf_get_fd_opts structure and leave open_flags to zero.

Finally, introduce bpf_prog_get_fd_by_id_opts(), to let the caller pass the
newly introduced data structure. Keep the existing bpf_prog_get_fd_by_id(),
and call bpf_prog_get_fd_by_id_opts() with NULL as opts argument.

Currently, setting permissions in the data structure has no effect, as the
kernel does not evaluate them. The new variant has been introduced anyway
for symmetry with bpf_map_get_fd_by_id_opts(). Evaluating permissions could
be done in future kernel versions.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/bpf.c             | 37 ++++++++++++++++++++++++++++++++-
 tools/lib/bpf/bpf.h             | 11 ++++++++++
 tools/lib/bpf/libbpf.c          |  4 ++++
 tools/lib/bpf/libbpf.map        |  1 +
 tools/lib/bpf/libbpf_internal.h |  3 +++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 5eb0df90eb2b..9014a61bca83 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -910,18 +910,53 @@ int bpf_link_get_next_id(__u32 start_id, __u32 *next_id)
 	return bpf_obj_get_next_id(start_id, next_id, BPF_LINK_GET_NEXT_ID);
 }
 
-int bpf_prog_get_fd_by_id(__u32 id)
+static int
+bpf_prog_get_fd_by_id_opts_check(__u32 id, const struct bpf_get_fd_opts *opts,
+				 bool check_support)
 {
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.prog_id = id;
+	if (!check_support ||
+	    kernel_supports(NULL, FEAT_GET_FD_BY_ID_OPEN_FLAGS))
+		attr.open_flags = OPTS_GET(opts, flags, 0);
 
 	fd = sys_bpf_fd(BPF_PROG_GET_FD_BY_ID, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
 
+int bpf_prog_get_fd_by_id_opts(__u32 id, const struct bpf_get_fd_opts *opts)
+{
+	return bpf_prog_get_fd_by_id_opts_check(id, opts, true);
+}
+
+int probe_get_fd_by_id_open_flags(void)
+{
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
+	int ret, fd;
+
+	fd = bpf_prog_get_fd_by_id_opts_check(0, &opts, false);
+	ret = (fd >= 0) || (errno == ENOENT);
+
+	if (fd >= 0)
+		close(fd);
+
+	return ret;
+}
+
+int bpf_prog_get_fd_by_id(__u32 id)
+{
+	return bpf_prog_get_fd_by_id_opts(id, NULL);
+}
+
 int bpf_map_get_fd_by_id(__u32 id)
 {
 	union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 88a7cc4bd76f..bc241343a0f9 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -271,6 +271,14 @@ LIBBPF_API int bpf_map_update_batch(int fd, const void *keys, const void *values
 				    const struct bpf_map_batch_opts *opts);
 
 LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
+
+struct bpf_get_fd_opts {
+	size_t sz; /* size of this struct for forward/backward compatibility */
+	__u32 flags; /* permissions requested for the operation on fd */
+	__u32 :0;
+};
+#define bpf_get_fd_opts__last_field flags
+
 LIBBPF_API int bpf_obj_get(const char *pathname);
 
 struct bpf_prog_attach_opts {
@@ -354,6 +362,9 @@ LIBBPF_API int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id);
 LIBBPF_API int bpf_map_get_next_id(__u32 start_id, __u32 *next_id);
 LIBBPF_API int bpf_btf_get_next_id(__u32 start_id, __u32 *next_id);
 LIBBPF_API int bpf_link_get_next_id(__u32 start_id, __u32 *next_id);
+
+LIBBPF_API int bpf_prog_get_fd_by_id_opts(__u32 id,
+					  const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_prog_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_map_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_btf_get_fd_by_id(__u32 id);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b01fe01b0761..12b5e6833726 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4784,6 +4784,10 @@ static struct kern_feature_desc {
 	[FEAT_SYSCALL_WRAPPER] = {
 		"Kernel using syscall wrapper", probe_kern_syscall_wrapper,
 	},
+	[FEAT_GET_FD_BY_ID_OPEN_FLAGS] = {
+		"open_flags accepted for bpf_*_get_fd_by_id() funcs",
+		probe_get_fd_by_id_open_flags,
+	},
 };
 
 bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 0625adb9e888..ab818612a585 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -364,4 +364,5 @@ LIBBPF_1.0.0 {
 		libbpf_bpf_map_type_str;
 		libbpf_bpf_prog_type_str;
 		perf_buffer__buffer;
+		bpf_prog_get_fd_by_id_opts;
 };
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4135ae0a2bc3..84c589b310e7 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -354,10 +354,13 @@ enum kern_feature_id {
 	FEAT_BTF_ENUM64,
 	/* Kernel uses syscall wrapper (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) */
 	FEAT_SYSCALL_WRAPPER,
+	/* open_flags accepted for bpf_*_get_fd_by_id() funcs */
+	FEAT_GET_FD_BY_ID_OPEN_FLAGS,
 	__FEAT_CNT,
 };
 
 int probe_memcg_account(void);
+int probe_get_fd_by_id_open_flags(void);
 bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id);
 int bump_rlimit_memlock(void);
 
-- 
2.25.1

[RFC][PATCH v3 04/15] libbpf: Introduce bpf_map_get_fd_by_id_opts()

[Roberto Sassu]

Introduce bpf_map_get_fd_by_id_opts(), to let the caller pass a
bpf_get_fd_opts structure with flags set to the permissions necessary to
perform the operations on the obtained file descriptor.

Don't check FEAT_GET_FD_BY_ID_OPEN_FLAGS, as current kernels already take
open_flags as last bpf_attr field for this request.

Keep the existing bpf_map_get_fd_by_id(), and call
bpf_map_get_fd_by_id_opts() with NULL as opts argument, to request
read-write permissions.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/bpf.c      | 12 +++++++++++-
 tools/lib/bpf/bpf.h      |  2 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 9014a61bca83..4b574ad046f3 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -957,18 +957,28 @@ int bpf_prog_get_fd_by_id(__u32 id)
 	return bpf_prog_get_fd_by_id_opts(id, NULL);
 }
 
-int bpf_map_get_fd_by_id(__u32 id)
+int bpf_map_get_fd_by_id_opts(__u32 id,
+			      const struct bpf_get_fd_opts *opts)
 {
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.map_id = id;
+	attr.open_flags = OPTS_GET(opts, flags, 0);
 
 	fd = sys_bpf_fd(BPF_MAP_GET_FD_BY_ID, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
 
+int bpf_map_get_fd_by_id(__u32 id)
+{
+	return bpf_map_get_fd_by_id_opts(id, NULL);
+}
+
 int bpf_btf_get_fd_by_id(__u32 id)
 {
 	union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index bc241343a0f9..d4b84d3f7e16 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -366,6 +366,8 @@ LIBBPF_API int bpf_link_get_next_id(__u32 start_id, __u32 *next_id);
 LIBBPF_API int bpf_prog_get_fd_by_id_opts(__u32 id,
 					  const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_prog_get_fd_by_id(__u32 id);
+LIBBPF_API int bpf_map_get_fd_by_id_opts(__u32 id,
+					 const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_map_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_btf_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_link_get_fd_by_id(__u32 id);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index ab818612a585..83dc18b5e5cf 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -365,4 +365,5 @@ LIBBPF_1.0.0 {
 		libbpf_bpf_prog_type_str;
 		perf_buffer__buffer;
 		bpf_prog_get_fd_by_id_opts;
+		bpf_map_get_fd_by_id_opts;
 };
-- 
2.25.1

[RFC][PATCH v3 05/15] libbpf: Introduce bpf_btf_get_fd_by_id_opts()

[Roberto Sassu]

Introduce bpf_btf_get_fd_by_id_opts(), to let the caller specify the needed
permissions for the operation. Keep the existing bpf_btf_get_fd_by_id()
function, to request read-write permissions.

Currently, setting permissions in the data structure has no effect, as the
kernel does not evaluate them. The new variant has been introduced anyway
for symmetry with bpf_map_get_fd_by_id_opts(). Evaluating permissions could
be done in future kernel versions.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/bpf.c      | 13 ++++++++++++-
 tools/lib/bpf/bpf.h      |  2 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 4b574ad046f3..b131bbc8509e 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -979,18 +979,29 @@ int bpf_map_get_fd_by_id(__u32 id)
 	return bpf_map_get_fd_by_id_opts(id, NULL);
 }
 
-int bpf_btf_get_fd_by_id(__u32 id)
+int bpf_btf_get_fd_by_id_opts(__u32 id,
+			      const struct bpf_get_fd_opts *opts)
 {
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.btf_id = id;
+	if (kernel_supports(NULL, FEAT_GET_FD_BY_ID_OPEN_FLAGS))
+		attr.open_flags = OPTS_GET(opts, flags, 0);
 
 	fd = sys_bpf_fd(BPF_BTF_GET_FD_BY_ID, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
 
+int bpf_btf_get_fd_by_id(__u32 id)
+{
+	return bpf_btf_get_fd_by_id_opts(id, NULL);
+}
+
 int bpf_link_get_fd_by_id(__u32 id)
 {
 	union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index d4b84d3f7e16..45d3739d901f 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -369,6 +369,8 @@ LIBBPF_API int bpf_prog_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_map_get_fd_by_id_opts(__u32 id,
 					 const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_map_get_fd_by_id(__u32 id);
+LIBBPF_API int bpf_btf_get_fd_by_id_opts(__u32 id,
+					 const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_btf_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_link_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 83dc18b5e5cf..ee6755dbf004 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -366,4 +366,5 @@ LIBBPF_1.0.0 {
 		perf_buffer__buffer;
 		bpf_prog_get_fd_by_id_opts;
 		bpf_map_get_fd_by_id_opts;
+		bpf_btf_get_fd_by_id_opts;
 };
-- 
2.25.1

[RFC][PATCH v3 06/15] libbpf: Introduce bpf_link_get_fd_by_id_opts()

[Roberto Sassu]

Introduce bpf_link_get_fd_by_id_opts(), to let the caller specify the
needed permissions for the operation. Keep the existing
bpf_link_get_fd_by_id(), to request read-write permissions.

Currently, setting permissions in the data structure has no effect, as the
kernel does not evaluate them. The new variant has been introduced anyway
for symmetry with bpf_map_get_fd_by_id_opts(). Evaluating permissions could
be done in future kernel versions.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/bpf.c      | 13 ++++++++++++-
 tools/lib/bpf/bpf.h      |  2 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index b131bbc8509e..5f2785a4c358 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -1002,18 +1002,29 @@ int bpf_btf_get_fd_by_id(__u32 id)
 	return bpf_btf_get_fd_by_id_opts(id, NULL);
 }
 
-int bpf_link_get_fd_by_id(__u32 id)
+int bpf_link_get_fd_by_id_opts(__u32 id,
+			       const struct bpf_get_fd_opts *opts)
 {
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.link_id = id;
+	if (kernel_supports(NULL, FEAT_GET_FD_BY_ID_OPEN_FLAGS))
+		attr.open_flags = OPTS_GET(opts, flags, 0);
 
 	fd = sys_bpf_fd(BPF_LINK_GET_FD_BY_ID, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
 
+int bpf_link_get_fd_by_id(__u32 id)
+{
+	return bpf_link_get_fd_by_id_opts(id, NULL);
+}
+
 int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len)
 {
 	union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 45d3739d901f..b75fd9d37bad 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -372,6 +372,8 @@ LIBBPF_API int bpf_map_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_btf_get_fd_by_id_opts(__u32 id,
 					 const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_btf_get_fd_by_id(__u32 id);
+LIBBPF_API int bpf_link_get_fd_by_id_opts(__u32 id,
+					  const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_link_get_fd_by_id(__u32 id);
 LIBBPF_API int bpf_obj_get_info_by_fd(int bpf_fd, void *info, __u32 *info_len);
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index ee6755dbf004..dba97d2ef8a9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -367,4 +367,5 @@ LIBBPF_1.0.0 {
 		bpf_prog_get_fd_by_id_opts;
 		bpf_map_get_fd_by_id_opts;
 		bpf_btf_get_fd_by_id_opts;
+		bpf_link_get_fd_by_id_opts;
 };
-- 
2.25.1

[RFC][PATCH v3 07/15] libbpf: Introduce bpf_obj_get_opts()

[Roberto Sassu]

Introduce bpf_obj_get_opts(), to let the caller pass the needed permissions
for the operation. Keep the existing bpf_obj_get() to request read-write
permissions.

bpf_obj_get() allows the caller to get a file descriptor from a pinned
object with the provided pathname. Specifying permissions has only effect
on maps (for links, the permission must be always read-write).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/bpf.c      | 12 +++++++++++-
 tools/lib/bpf/bpf.h      |  2 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 5f2785a4c358..0df088890864 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -577,18 +577,28 @@ int bpf_obj_pin(int fd, const char *pathname)
 	return libbpf_err_errno(ret);
 }
 
-int bpf_obj_get(const char *pathname)
+int bpf_obj_get_opts(const char *pathname,
+		     const struct bpf_get_fd_opts *opts)
 {
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, sizeof(attr));
 	attr.pathname = ptr_to_u64((void *)pathname);
+	attr.file_flags = OPTS_GET(opts, flags, 0);
 
 	fd = sys_bpf_fd(BPF_OBJ_GET, &attr, sizeof(attr));
 	return libbpf_err_errno(fd);
 }
 
+int bpf_obj_get(const char *pathname)
+{
+	return bpf_obj_get_opts(pathname, NULL);
+}
+
 int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
 		    unsigned int flags)
 {
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index b75fd9d37bad..e0c5018cc131 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -279,6 +279,8 @@ struct bpf_get_fd_opts {
 };
 #define bpf_get_fd_opts__last_field flags
 
+LIBBPF_API int bpf_obj_get_opts(const char *pathname,
+				const struct bpf_get_fd_opts *opts);
 LIBBPF_API int bpf_obj_get(const char *pathname);
 
 struct bpf_prog_attach_opts {
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index dba97d2ef8a9..55516b19e22f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -368,4 +368,5 @@ LIBBPF_1.0.0 {
 		bpf_map_get_fd_by_id_opts;
 		bpf_btf_get_fd_by_id_opts;
 		bpf_link_get_fd_by_id_opts;
+		bpf_obj_get_opts;
 };
-- 
2.25.1

[RFC][PATCH v3 08/15] bpftool: Add opts parameter to open_obj_pinned_any() and open_obj_pinned()

[Roberto Sassu]

Start to propagate the opts parameter from libbpf up to the bpftool
subcommand implementation, so that the appropriate permissions can be
requested depending on the operation type.

Add the bpf_get_fd_opts structure as parameter to open_obj_pinned_any() and
open_obj_pinned(), so that it can be sent to the new libbpf function
bpf_obj_get_opts().

Pass NULL as argument value, so that there is no change in the current
behavior.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/common.c | 16 +++++++++-------
 tools/bpf/bpftool/link.c   |  2 +-
 tools/bpf/bpftool/main.c   |  1 -
 tools/bpf/bpftool/main.h   |  7 +++++--
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 067e9ea59e3b..d3e3c779b027 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -191,7 +191,8 @@ int mount_tracefs(const char *target)
 	return err;
 }
 
-int open_obj_pinned(const char *path, bool quiet)
+int open_obj_pinned(const char *path, bool quiet,
+		    const struct bpf_get_fd_opts *opts)
 {
 	char *pname;
 	int fd = -1;
@@ -203,7 +204,7 @@ int open_obj_pinned(const char *path, bool quiet)
 		goto out_ret;
 	}
 
-	fd = bpf_obj_get(pname);
+	fd = bpf_obj_get_opts(pname, opts);
 	if (fd < 0) {
 		if (!quiet)
 			p_err("bpf obj get (%s): %s", pname,
@@ -219,12 +220,13 @@ int open_obj_pinned(const char *path, bool quiet)
 	return fd;
 }
 
-int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type)
+int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type,
+			const struct bpf_get_fd_opts *opts)
 {
 	enum bpf_obj_type type;
 	int fd;
 
-	fd = open_obj_pinned(path, false);
+	fd = open_obj_pinned(path, false, opts);
 	if (fd < 0)
 		return -1;
 
@@ -474,7 +476,7 @@ static int do_build_table_cb(const char *fpath, const struct stat *sb,
 	if (typeflag != FTW_F)
 		goto out_ret;
 
-	fd = open_obj_pinned(fpath, true);
+	fd = open_obj_pinned(fpath, true, NULL);
 	if (fd < 0)
 		goto out_ret;
 
@@ -835,7 +837,7 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 		path = **argv;
 		NEXT_ARGP();
 
-		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_PROG);
+		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_PROG, NULL);
 		if ((*fds)[0] < 0)
 			return -1;
 		return 1;
@@ -972,7 +974,7 @@ int map_parse_fds(int *argc, char ***argv, int **fds)
 		path = **argv;
 		NEXT_ARGP();
 
-		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_MAP);
+		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_MAP, NULL);
 		if ((*fds)[0] < 0)
 			return -1;
 		return 1;
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 7a20931c3250..62902d5ea1ae 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -44,7 +44,7 @@ static int link_parse_fd(int *argc, char ***argv)
 		path = **argv;
 		NEXT_ARGP();
 
-		return open_obj_pinned_any(path, BPF_OBJ_LINK);
+		return open_obj_pinned_any(path, BPF_OBJ_LINK, NULL);
 	}
 
 	p_err("expected 'id' or 'pinned', got: '%s'?", **argv);
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 451cefc2d0da..c0f783f0f3b7 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -9,7 +9,6 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include <bpf/bpf.h>
 #include <bpf/btf.h>
 #include <bpf/hashmap.h>
 #include <bpf/libbpf.h>
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 5e5060c2ac04..53dfcb91ecac 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -9,6 +9,7 @@
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <bpf/bpf.h>
 #include <linux/bpf.h>
 #include <linux/compiler.h>
 #include <linux/kernel.h>
@@ -141,8 +142,10 @@ void get_prog_full_name(const struct bpf_prog_info *prog_info, int prog_fd,
 int get_fd_type(int fd);
 const char *get_fd_type_name(enum bpf_obj_type type);
 char *get_fdinfo(int fd, const char *key);
-int open_obj_pinned(const char *path, bool quiet);
-int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type);
+int open_obj_pinned(const char *path, bool quiet,
+		    const struct bpf_get_fd_opts *opts);
+int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type,
+			const struct bpf_get_fd_opts *opts);
 int mount_bpffs_for_pin(const char *name);
 int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(int *, char ***));
 int do_pin_fd(int fd, const char *name);
-- 
2.25.1

[RFC][PATCH v3 09/15] bpftool: Add opts parameter to *_parse_fd() functions

[Roberto Sassu]

Add the opts parameter to map_parse_fd(), prog_parse_fd(), link_parse_fd()
and btf_parse_fd() at the same time, as those functions are passed to
do_pin_any().

Pass NULL to those functions, so that the current behavior does not change,
and adjust permissions in a later patch.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/btf.c    |  9 +++++----
 tools/bpf/bpftool/cgroup.c |  4 ++--
 tools/bpf/bpftool/common.c | 12 +++++++-----
 tools/bpf/bpftool/iter.c   |  2 +-
 tools/bpf/bpftool/link.c   | 11 ++++++-----
 tools/bpf/bpftool/main.h   |  9 ++++++---
 tools/bpf/bpftool/map.c    |  6 +++---
 tools/bpf/bpftool/net.c    |  2 +-
 tools/bpf/bpftool/prog.c   | 10 +++++-----
 9 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 0744bd1150be..ab7bf9f3cc27 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -610,7 +610,7 @@ static int do_dump(int argc, char **argv)
 			return -1;
 		}
 
-		fd = prog_parse_fd(&argc, &argv);
+		fd = prog_parse_fd(&argc, &argv, NULL);
 		if (fd < 0)
 			return -1;
 
@@ -712,7 +712,8 @@ static int do_dump(int argc, char **argv)
 	return err;
 }
 
-static int btf_parse_fd(int *argc, char ***argv)
+static int btf_parse_fd(int *argc, char ***argv,
+			const struct bpf_get_fd_opts *opts)
 {
 	unsigned int id;
 	char *endptr;
@@ -731,7 +732,7 @@ static int btf_parse_fd(int *argc, char ***argv)
 	}
 	NEXT_ARGP();
 
-	fd = bpf_btf_get_fd_by_id(id);
+	fd = bpf_btf_get_fd_by_id_opts(id, opts);
 	if (fd < 0)
 		p_err("can't get BTF object by id (%u): %s",
 		      id, strerror(errno));
@@ -982,7 +983,7 @@ static int do_show(int argc, char **argv)
 	__u32 id = 0;
 
 	if (argc == 2) {
-		fd = btf_parse_fd(&argc, &argv);
+		fd = btf_parse_fd(&argc, &argv, NULL);
 		if (fd < 0)
 			return -1;
 	}
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index cced668fb2a3..cc007ddc945b 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -490,7 +490,7 @@ static int do_attach(int argc, char **argv)
 
 	argc -= 2;
 	argv = &argv[2];
-	prog_fd = prog_parse_fd(&argc, &argv);
+	prog_fd = prog_parse_fd(&argc, &argv, NULL);
 	if (prog_fd < 0)
 		goto exit_cgroup;
 
@@ -548,7 +548,7 @@ static int do_detach(int argc, char **argv)
 
 	argc -= 2;
 	argv = &argv[2];
-	prog_fd = prog_parse_fd(&argc, &argv);
+	prog_fd = prog_parse_fd(&argc, &argv, NULL);
 	if (prog_fd < 0)
 		goto exit_cgroup;
 
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index d3e3c779b027..17da60c83b7c 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -297,12 +297,13 @@ int do_pin_fd(int fd, const char *name)
 	return err;
 }
 
-int do_pin_any(int argc, char **argv, int (*get_fd)(int *, char ***))
+int do_pin_any(int argc, char **argv,
+	       int (*get_fd)(int *, char ***, const struct bpf_get_fd_opts *))
 {
 	int err;
 	int fd;
 
-	fd = get_fd(&argc, &argv);
+	fd = get_fd(&argc, &argv, NULL);
 	if (fd < 0)
 		return fd;
 
@@ -847,7 +848,8 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 	return -1;
 }
 
-int prog_parse_fd(int *argc, char ***argv)
+int prog_parse_fd(int *argc, char ***argv,
+		  const struct bpf_get_fd_opts *opts)
 {
 	int *fds = NULL;
 	int nb_fds, fd;
@@ -984,7 +986,7 @@ int map_parse_fds(int *argc, char ***argv, int **fds)
 	return -1;
 }
 
-int map_parse_fd(int *argc, char ***argv)
+int map_parse_fd(int *argc, char ***argv, const struct bpf_get_fd_opts *opts)
 {
 	int *fds = NULL;
 	int nb_fds, fd;
@@ -1016,7 +1018,7 @@ int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len)
 	int err;
 	int fd;
 
-	fd = map_parse_fd(argc, argv);
+	fd = map_parse_fd(argc, argv, NULL);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c
index f88fdc820d23..1412be9eb298 100644
--- a/tools/bpf/bpftool/iter.c
+++ b/tools/bpf/bpftool/iter.c
@@ -34,7 +34,7 @@ static int do_pin(int argc, char **argv)
 				return -1;
 			}
 
-			map_fd = map_parse_fd(&argc, &argv);
+			map_fd = map_parse_fd(&argc, &argv, NULL);
 			if (map_fd < 0)
 				return -1;
 
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 62902d5ea1ae..b319f9769d90 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -15,7 +15,8 @@
 
 static struct hashmap *link_table;
 
-static int link_parse_fd(int *argc, char ***argv)
+static int link_parse_fd(int *argc, char ***argv,
+			 const struct bpf_get_fd_opts *opts)
 {
 	int fd;
 
@@ -32,7 +33,7 @@ static int link_parse_fd(int *argc, char ***argv)
 		}
 		NEXT_ARGP();
 
-		fd = bpf_link_get_fd_by_id(id);
+		fd = bpf_link_get_fd_by_id_opts(id, opts);
 		if (fd < 0)
 			p_err("failed to get link with ID %d: %s", id, strerror(errno));
 		return fd;
@@ -44,7 +45,7 @@ static int link_parse_fd(int *argc, char ***argv)
 		path = **argv;
 		NEXT_ARGP();
 
-		return open_obj_pinned_any(path, BPF_OBJ_LINK, NULL);
+		return open_obj_pinned_any(path, BPF_OBJ_LINK, opts);
 	}
 
 	p_err("expected 'id' or 'pinned', got: '%s'?", **argv);
@@ -321,7 +322,7 @@ static int do_show(int argc, char **argv)
 	build_obj_refs_table(&refs_table, BPF_OBJ_LINK);
 
 	if (argc == 2) {
-		fd = link_parse_fd(&argc, &argv);
+		fd = link_parse_fd(&argc, &argv, NULL);
 		if (fd < 0)
 			return fd;
 		return do_show_link(fd);
@@ -385,7 +386,7 @@ static int do_detach(int argc, char **argv)
 		return 1;
 	}
 
-	fd = link_parse_fd(&argc, &argv);
+	fd = link_parse_fd(&argc, &argv, NULL);
 	if (fd < 0)
 		return 1;
 
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 53dfcb91ecac..88218add9e9f 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -147,7 +147,8 @@ int open_obj_pinned(const char *path, bool quiet,
 int open_obj_pinned_any(const char *path, enum bpf_obj_type exp_type,
 			const struct bpf_get_fd_opts *opts);
 int mount_bpffs_for_pin(const char *name);
-int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(int *, char ***));
+int do_pin_any(int argc, char **argv,
+	       int (*get_fd)(int *, char ***, const struct bpf_get_fd_opts *));
 int do_pin_fd(int fd, const char *name);
 
 /* commands available in bootstrap mode */
@@ -168,9 +169,11 @@ int do_struct_ops(int argc, char **argv) __weak;
 int do_iter(int argc, char **argv) __weak;
 
 int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
-int prog_parse_fd(int *argc, char ***argv);
+int prog_parse_fd(int *argc, char ***argv,
+		  const struct bpf_get_fd_opts *opts);
 int prog_parse_fds(int *argc, char ***argv, int **fds);
-int map_parse_fd(int *argc, char ***argv);
+int map_parse_fd(int *argc, char ***argv,
+		 const struct bpf_get_fd_opts *opts);
 int map_parse_fds(int *argc, char ***argv, int **fds);
 int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
 
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 38b6bc9c26c3..bacdb17f0c6d 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -381,7 +381,7 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 				return -1;
 			}
 
-			fd = map_parse_fd(&argc, &argv);
+			fd = map_parse_fd(&argc, &argv, NULL);
 			if (fd < 0)
 				return -1;
 
@@ -402,7 +402,7 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 				p_info("Warning: updating program array via MAP_ID, make sure this map is kept open\n"
 				       "         by some process or pinned otherwise update will be lost");
 
-			fd = prog_parse_fd(&argc, &argv);
+			fd = prog_parse_fd(&argc, &argv, NULL);
 			if (fd < 0)
 				return -1;
 
@@ -1399,7 +1399,7 @@ static int do_freeze(int argc, char **argv)
 	if (!REQ_ARGS(2))
 		return -1;
 
-	fd = map_parse_fd(&argc, &argv);
+	fd = map_parse_fd(&argc, &argv, NULL);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index 526a332c48e6..4c8a2831e04f 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -571,7 +571,7 @@ static int do_attach(int argc, char **argv)
 	}
 	NEXT_ARG();
 
-	progfd = prog_parse_fd(&argc, &argv);
+	progfd = prog_parse_fd(&argc, &argv, NULL);
 	if (progfd < 0)
 		return -EINVAL;
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index f081de398b60..400dfc085ee5 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1027,7 +1027,7 @@ static int parse_attach_detach_args(int argc, char **argv, int *progfd,
 	if (!REQ_ARGS(3))
 		return -EINVAL;
 
-	*progfd = prog_parse_fd(&argc, &argv);
+	*progfd = prog_parse_fd(&argc, &argv, NULL);
 	if (*progfd < 0)
 		return *progfd;
 
@@ -1046,7 +1046,7 @@ static int parse_attach_detach_args(int argc, char **argv, int *progfd,
 	if (!REQ_ARGS(2))
 		return -EINVAL;
 
-	*mapfd = map_parse_fd(&argc, &argv);
+	*mapfd = map_parse_fd(&argc, &argv, NULL);
 	if (*mapfd < 0)
 		return *mapfd;
 
@@ -1270,7 +1270,7 @@ static int do_run(int argc, char **argv)
 	if (!REQ_ARGS(4))
 		return -1;
 
-	fd = prog_parse_fd(&argc, &argv);
+	fd = prog_parse_fd(&argc, &argv, NULL);
 	if (fd < 0)
 		return -1;
 
@@ -1542,7 +1542,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 			}
 			NEXT_ARG();
 
-			fd = map_parse_fd(&argc, &argv);
+			fd = map_parse_fd(&argc, &argv, NULL);
 			if (fd < 0)
 				goto err_free_reuse_maps;
 
@@ -2233,7 +2233,7 @@ static int do_profile(int argc, char **argv)
 		return -EINVAL;
 
 	/* parse target fd */
-	profile_tgt_fd = prog_parse_fd(&argc, &argv);
+	profile_tgt_fd = prog_parse_fd(&argc, &argv, NULL);
 	if (profile_tgt_fd < 0) {
 		p_err("failed to parse fd");
 		return -1;
-- 
2.25.1

[RFC][PATCH v3 10/15] bpftool: Add opts parameter to *_parse_fds()

[Roberto Sassu]

Add the opts parameter to prog_parse_fds(), map_parse_fds(), and the static
functions called by them, respectively prog_fd_by_nametag() and
map_fd_by_name().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/common.c | 34 +++++++++++++++++++---------------
 tools/bpf/bpftool/main.c   |  1 +
 tools/bpf/bpftool/main.h   |  6 ++++--
 tools/bpf/bpftool/map.c    |  4 ++--
 tools/bpf/bpftool/prog.c   |  4 ++--
 5 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 17da60c83b7c..913771fcf8ec 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -723,7 +723,8 @@ print_all_levels(__maybe_unused enum libbpf_print_level level,
 	return vfprintf(stderr, format, args);
 }
 
-static int prog_fd_by_nametag(void *nametag, int **fds, bool tag)
+static int prog_fd_by_nametag(void *nametag, int **fds, bool tag,
+			      const struct bpf_get_fd_opts *opts)
 {
 	unsigned int id = 0;
 	int fd, nb_fds = 0;
@@ -743,7 +744,7 @@ static int prog_fd_by_nametag(void *nametag, int **fds, bool tag)
 			return nb_fds;
 		}
 
-		fd = bpf_prog_get_fd_by_id(id);
+		fd = bpf_prog_get_fd_by_id_opts(id, opts);
 		if (fd < 0) {
 			p_err("can't get prog by id (%u): %s",
 			      id, strerror(errno));
@@ -782,7 +783,8 @@ static int prog_fd_by_nametag(void *nametag, int **fds, bool tag)
 	return -1;
 }
 
-int prog_parse_fds(int *argc, char ***argv, int **fds)
+int prog_parse_fds(int *argc, char ***argv, int **fds,
+		   const struct bpf_get_fd_opts *opts)
 {
 	if (is_prefix(**argv, "id")) {
 		unsigned int id;
@@ -797,7 +799,7 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 		}
 		NEXT_ARGP();
 
-		(*fds)[0] = bpf_prog_get_fd_by_id(id);
+		(*fds)[0] = bpf_prog_get_fd_by_id_opts(id, opts);
 		if ((*fds)[0] < 0) {
 			p_err("get by id (%u): %s", id, strerror(errno));
 			return -1;
@@ -816,7 +818,7 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 		}
 		NEXT_ARGP();
 
-		return prog_fd_by_nametag(tag, fds, true);
+		return prog_fd_by_nametag(tag, fds, true, opts);
 	} else if (is_prefix(**argv, "name")) {
 		char *name;
 
@@ -829,7 +831,7 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 		}
 		NEXT_ARGP();
 
-		return prog_fd_by_nametag(name, fds, false);
+		return prog_fd_by_nametag(name, fds, false, opts);
 	} else if (is_prefix(**argv, "pinned")) {
 		char *path;
 
@@ -838,7 +840,7 @@ int prog_parse_fds(int *argc, char ***argv, int **fds)
 		path = **argv;
 		NEXT_ARGP();
 
-		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_PROG, NULL);
+		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_PROG, opts);
 		if ((*fds)[0] < 0)
 			return -1;
 		return 1;
@@ -859,7 +861,7 @@ int prog_parse_fd(int *argc, char ***argv,
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = prog_parse_fds(argc, argv, &fds);
+	nb_fds = prog_parse_fds(argc, argv, &fds, opts);
 	if (nb_fds != 1) {
 		if (nb_fds > 1) {
 			p_err("several programs match this handle");
@@ -876,7 +878,8 @@ int prog_parse_fd(int *argc, char ***argv,
 	return fd;
 }
 
-static int map_fd_by_name(char *name, int **fds)
+static int map_fd_by_name(char *name, int **fds,
+			  const struct bpf_get_fd_opts *opts)
 {
 	unsigned int id = 0;
 	int fd, nb_fds = 0;
@@ -896,7 +899,7 @@ static int map_fd_by_name(char *name, int **fds)
 			return nb_fds;
 		}
 
-		fd = bpf_map_get_fd_by_id(id);
+		fd = bpf_map_get_fd_by_id_opts(id, opts);
 		if (fd < 0) {
 			p_err("can't get map by id (%u): %s",
 			      id, strerror(errno));
@@ -934,7 +937,8 @@ 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,
+		  const struct bpf_get_fd_opts *opts)
 {
 	if (is_prefix(**argv, "id")) {
 		unsigned int id;
@@ -949,7 +953,7 @@ int map_parse_fds(int *argc, char ***argv, int **fds)
 		}
 		NEXT_ARGP();
 
-		(*fds)[0] = bpf_map_get_fd_by_id(id);
+		(*fds)[0] = bpf_map_get_fd_by_id_opts(id, opts);
 		if ((*fds)[0] < 0) {
 			p_err("get map by id (%u): %s", id, strerror(errno));
 			return -1;
@@ -967,7 +971,7 @@ int map_parse_fds(int *argc, char ***argv, int **fds)
 		}
 		NEXT_ARGP();
 
-		return map_fd_by_name(name, fds);
+		return map_fd_by_name(name, fds, opts);
 	} else if (is_prefix(**argv, "pinned")) {
 		char *path;
 
@@ -976,7 +980,7 @@ int map_parse_fds(int *argc, char ***argv, int **fds)
 		path = **argv;
 		NEXT_ARGP();
 
-		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_MAP, NULL);
+		(*fds)[0] = open_obj_pinned_any(path, BPF_OBJ_MAP, opts);
 		if ((*fds)[0] < 0)
 			return -1;
 		return 1;
@@ -996,7 +1000,7 @@ int map_parse_fd(int *argc, char ***argv, const struct bpf_get_fd_opts *opts)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = map_parse_fds(argc, argv, &fds);
+	nb_fds = map_parse_fds(argc, argv, &fds, opts);
 	if (nb_fds != 1) {
 		if (nb_fds > 1) {
 			p_err("several maps match this handle");
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index c0f783f0f3b7..451cefc2d0da 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <bpf/bpf.h>
 #include <bpf/btf.h>
 #include <bpf/hashmap.h>
 #include <bpf/libbpf.h>
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 88218add9e9f..13125171ac8a 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -171,10 +171,12 @@ int do_iter(int argc, char **argv) __weak;
 int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
 int prog_parse_fd(int *argc, char ***argv,
 		  const struct bpf_get_fd_opts *opts);
-int prog_parse_fds(int *argc, char ***argv, int **fds);
+int prog_parse_fds(int *argc, char ***argv, int **fds,
+		   const struct bpf_get_fd_opts *opts);
 int map_parse_fd(int *argc, char ***argv,
 		 const struct bpf_get_fd_opts *opts);
-int map_parse_fds(int *argc, char ***argv, int **fds);
+int map_parse_fds(int *argc, char ***argv, int **fds,
+		  const struct bpf_get_fd_opts *opts);
 int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
 
 struct bpf_prog_linfo;
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index bacdb17f0c6d..d921cb1a116d 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -634,7 +634,7 @@ static int do_show_subset(int argc, char **argv)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = map_parse_fds(&argc, &argv, &fds);
+	nb_fds = map_parse_fds(&argc, &argv, &fds, NULL);
 	if (nb_fds < 1)
 		goto exit_free;
 
@@ -910,7 +910,7 @@ static int do_dump(int argc, char **argv)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = map_parse_fds(&argc, &argv, &fds);
+	nb_fds = map_parse_fds(&argc, &argv, &fds, NULL);
 	if (nb_fds < 1)
 		goto exit_free;
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 400dfc085ee5..e37af694152e 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -605,7 +605,7 @@ static int do_show_subset(int argc, char **argv)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = prog_parse_fds(&argc, &argv, &fds);
+	nb_fds = prog_parse_fds(&argc, &argv, &fds, NULL);
 	if (nb_fds < 1)
 		goto exit_free;
 
@@ -905,7 +905,7 @@ static int do_dump(int argc, char **argv)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = prog_parse_fds(&argc, &argv, &fds);
+	nb_fds = prog_parse_fds(&argc, &argv, &fds, NULL);
 	if (nb_fds < 1)
 		goto exit_free;
 
-- 
2.25.1

[RFC][PATCH v3 11/15] bpftool: Add opts parameter to map_parse_fd_and_info()

[Roberto Sassu]

Add the opts parameter to map_parse_fd_and_info(), as this function is
called directly by the functions implementing bpftool subcommands, so that
it is possible to specify the right permissions for accessing a map.

Still pass a NULL pointer as argument, and adjust permissions in a later
patch.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/btf.c           |  2 +-
 tools/bpf/bpftool/common.c        |  5 +++--
 tools/bpf/bpftool/main.h          |  3 ++-
 tools/bpf/bpftool/map.c           | 12 ++++++------
 tools/bpf/bpftool/map_perf_ring.c |  3 ++-
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index ab7bf9f3cc27..c3059cb3196c 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -580,7 +580,7 @@ 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, NULL);
 		if (fd < 0)
 			return -1;
 
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 913771fcf8ec..8a2412fc4410 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -1017,12 +1017,13 @@ int map_parse_fd(int *argc, char ***argv, const struct bpf_get_fd_opts *opts)
 	return fd;
 }
 
-int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len)
+int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len,
+			  const struct bpf_get_fd_opts *opts)
 {
 	int err;
 	int fd;
 
-	fd = map_parse_fd(argc, argv, NULL);
+	fd = map_parse_fd(argc, argv, opts);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 13125171ac8a..6c35b2149a15 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -177,7 +177,8 @@ int map_parse_fd(int *argc, char ***argv,
 		 const struct bpf_get_fd_opts *opts);
 int map_parse_fds(int *argc, char ***argv, int **fds,
 		  const struct bpf_get_fd_opts *opts);
-int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
+int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len,
+			  const struct bpf_get_fd_opts *opts);
 
 struct bpf_prog_linfo;
 #ifdef HAVE_LIBBFD_SUPPORT
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index d921cb1a116d..bb94eb0dd9bf 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -998,7 +998,7 @@ static int do_update(int argc, char **argv)
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
 	if (fd < 0)
 		return -1;
 
@@ -1077,7 +1077,7 @@ static int do_lookup(int argc, char **argv)
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
 	if (fd < 0)
 		return -1;
 
@@ -1128,7 +1128,7 @@ static int do_getnext(int argc, char **argv)
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
 	if (fd < 0)
 		return -1;
 
@@ -1199,7 +1199,7 @@ static int do_delete(int argc, char **argv)
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
 	if (fd < 0)
 		return -1;
 
@@ -1311,7 +1311,7 @@ static int do_create(int argc, char **argv)
 			if (!REQ_ARGS(2))
 				usage();
 			inner_map_fd = map_parse_fd_and_info(&argc, &argv,
-							     &info, &len);
+							     &info, &len, NULL);
 			if (inner_map_fd < 0)
 				return -1;
 			attr.inner_map_fd = inner_map_fd;
@@ -1360,7 +1360,7 @@ static int do_pop_dequeue(int argc, char **argv)
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c
index 6b0c410152de..fa062db08c87 100644
--- a/tools/bpf/bpftool/map_perf_ring.c
+++ b/tools/bpf/bpftool/map_perf_ring.c
@@ -135,7 +135,8 @@ int do_event_pipe(int argc, char **argv)
 	int err, map_fd;
 
 	map_info_len = sizeof(map_info);
-	map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len);
+	map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len,
+				       NULL);
 	if (map_fd < 0)
 		return -1;
 
-- 
2.25.1

[RFC][PATCH v3 12/15] bpftool: Add opts parameter in struct_ops functions

[Roberto Sassu]

Propagate the opts parameter to struct_ops functions, so that the
appropriate permissions can be requested for each operation of the
struct_ops subcommand.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/struct_ops.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
index e08a6ff2866c..51667db3f55f 100644
--- a/tools/bpf/bpftool/struct_ops.c
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -130,7 +130,8 @@ static struct bpf_map_info *map_info_alloc(__u32 *alloc_len)
  *    -1: Error and the caller should abort the iteration.
  */
 static int get_next_struct_ops_map(const char *name, int *res_fd,
-				   struct bpf_map_info *info, __u32 info_len)
+				   struct bpf_map_info *info, __u32 info_len,
+				   const struct bpf_get_fd_opts *opts)
 {
 	__u32 id = info->id;
 	int err, fd;
@@ -144,7 +145,7 @@ static int get_next_struct_ops_map(const char *name, int *res_fd,
 			return -1;
 		}
 
-		fd = bpf_map_get_fd_by_id(id);
+		fd = bpf_map_get_fd_by_id_opts(id, opts);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
@@ -186,7 +187,8 @@ typedef int (*work_func)(int fd, const struct bpf_map_info *info, void *data,
  * Then call "func(fd, info, data, wtr)" on each struct_ops map found.
  */
 static struct res do_search(const char *name, work_func func, void *data,
-			    struct json_writer *wtr)
+			    struct json_writer *wtr,
+			    const struct bpf_get_fd_opts *opts)
 {
 	struct bpf_map_info *info;
 	struct res res = {};
@@ -201,7 +203,8 @@ static struct res do_search(const char *name, work_func func, void *data,
 
 	if (wtr)
 		jsonw_start_array(wtr);
-	while ((err = get_next_struct_ops_map(name, &fd, info, info_len)) == 1) {
+	while ((err = get_next_struct_ops_map(name, &fd, info, info_len,
+					      opts)) == 1) {
 		res.nr_maps++;
 		err = func(fd, info, data, wtr);
 		if (err)
@@ -235,7 +238,8 @@ static struct res do_search(const char *name, work_func func, void *data,
 }
 
 static struct res do_one_id(const char *id_str, work_func func, void *data,
-			    struct json_writer *wtr)
+			    struct json_writer *wtr,
+			    const struct bpf_get_fd_opts *opts)
 {
 	struct bpf_map_info *info;
 	struct res res = {};
@@ -251,7 +255,7 @@ static struct res do_one_id(const char *id_str, work_func func, void *data,
 		return res;
 	}
 
-	fd = bpf_map_get_fd_by_id(id);
+	fd = bpf_map_get_fd_by_id_opts(id, opts);
 	if (fd < 0) {
 		p_err("can't get map by id (%lu): %s", id, strerror(errno));
 		res.nr_errs++;
@@ -300,16 +304,17 @@ static struct res do_one_id(const char *id_str, work_func func, void *data,
 static struct res do_work_on_struct_ops(const char *search_type,
 					const char *search_term,
 					work_func func, void *data,
-					struct json_writer *wtr)
+					struct json_writer *wtr,
+					const struct bpf_get_fd_opts *opts)
 {
 	if (search_type) {
 		if (is_prefix(search_type, "id"))
-			return do_one_id(search_term, func, data, wtr);
+			return do_one_id(search_term, func, data, wtr, opts);
 		else if (!is_prefix(search_type, "name"))
 			usage();
 	}
 
-	return do_search(search_term, func, data, wtr);
+	return do_search(search_term, func, data, wtr, opts);
 }
 
 static int __do_show(int fd, const struct bpf_map_info *info, void *data,
@@ -344,7 +349,7 @@ static int do_show(int argc, char **argv)
 	}
 
 	res = do_work_on_struct_ops(search_type, search_term, __do_show,
-				    NULL, json_wtr);
+				    NULL, json_wtr, NULL);
 
 	return cmd_retval(&res, !!search_term);
 }
@@ -433,7 +438,7 @@ static int do_dump(int argc, char **argv)
 	d.prog_id_as_func_ptr = true;
 
 	res = do_work_on_struct_ops(search_type, search_term, __do_dump, &d,
-				    wtr);
+				    wtr, NULL);
 
 	if (!json_output)
 		jsonw_destroy(&wtr);
@@ -472,7 +477,7 @@ static int do_unregister(int argc, char **argv)
 	search_term = GET_ARG();
 
 	res = do_work_on_struct_ops(search_type, search_term,
-				    __do_unregister, NULL, NULL);
+				    __do_unregister, NULL, NULL, NULL);
 
 	return cmd_retval(&res, true);
 }
-- 
2.25.1

[RFC][PATCH v3 13/15] bpftool: Complete switch to bpf_*_get_fd_by_id_opts()

[Roberto Sassu]

Complete the switch to the new functions bpf_*_get_fd_by_id_opts(),
accepting the additional opts parameter.

The value passed to these functions is always NULL, so that there is no
variation in the behavior.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/btf.c        | 8 ++++----
 tools/bpf/bpftool/btf_dumper.c | 2 +-
 tools/bpf/bpftool/cgroup.c     | 4 ++--
 tools/bpf/bpftool/link.c       | 4 ++--
 tools/bpf/bpftool/map.c        | 2 +-
 tools/bpf/bpftool/prog.c       | 4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index c3059cb3196c..2cbc777f1520 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -536,7 +536,7 @@ static bool btf_is_kernel_module(__u32 btf_id)
 	__u32 len;
 	int err;
 
-	btf_fd = bpf_btf_get_fd_by_id(btf_id);
+	btf_fd = bpf_btf_get_fd_by_id_opts(btf_id, NULL);
 	if (btf_fd < 0) {
 		p_err("can't get BTF object by id (%u): %s", btf_id, strerror(errno));
 		return false;
@@ -779,10 +779,10 @@ build_btf_type_table(struct hashmap *tab, enum bpf_obj_type type,
 
 		switch (type) {
 		case BPF_OBJ_PROG:
-			fd = bpf_prog_get_fd_by_id(id);
+			fd = bpf_prog_get_fd_by_id_opts(id, NULL);
 			break;
 		case BPF_OBJ_MAP:
-			fd = bpf_map_get_fd_by_id(id);
+			fd = bpf_map_get_fd_by_id_opts(id, NULL);
 			break;
 		default:
 			err = -1;
@@ -1037,7 +1037,7 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
-		fd = bpf_btf_get_fd_by_id(id);
+		fd = bpf_btf_get_fd_by_id_opts(id, NULL);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 125798b0bc5d..8acc100c3093 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -53,7 +53,7 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
 		goto print;
 
 	/* Get the bpf_prog's name.  Obtain from func_info. */
-	prog_fd = bpf_prog_get_fd_by_id(prog_id);
+	prog_fd = bpf_prog_get_fd_by_id_opts(prog_id, NULL);
 	if (prog_fd < 0)
 		goto print;
 
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index cc007ddc945b..ce8cf97c190f 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -78,7 +78,7 @@ static void guess_vmlinux_btf_id(__u32 attach_btf_obj_id)
 	btf_info.name = ptr_to_u64(name);
 	btf_info.name_len = sizeof(name);
 
-	fd = bpf_btf_get_fd_by_id(attach_btf_obj_id);
+	fd = bpf_btf_get_fd_by_id_opts(attach_btf_obj_id, NULL);
 	if (fd < 0)
 		return;
 
@@ -104,7 +104,7 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
 	__u32 info_len = sizeof(info);
 	int prog_fd;
 
-	prog_fd = bpf_prog_get_fd_by_id(id);
+	prog_fd = bpf_prog_get_fd_by_id_opts(id, NULL);
 	if (prog_fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index b319f9769d90..9851c89f5798 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -99,7 +99,7 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
 	__u32 len = sizeof(*info);
 	int err, prog_fd;
 
-	prog_fd = bpf_prog_get_fd_by_id(prog_id);
+	prog_fd = bpf_prog_get_fd_by_id_opts(prog_id, NULL);
 	if (prog_fd < 0)
 		return prog_fd;
 
@@ -343,7 +343,7 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
-		fd = bpf_link_get_fd_by_id(id);
+		fd = bpf_link_get_fd_by_id_opts(id, NULL);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index bb94eb0dd9bf..103bd44cd851 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -702,7 +702,7 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
-		fd = bpf_map_get_fd_by_id(id);
+		fd = bpf_map_get_fd_by_id_opts(id, NULL);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e37af694152e..648546342988 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -251,7 +251,7 @@ static void *find_metadata(int prog_fd, struct bpf_map_info *map_info)
 		goto free_map_ids;
 
 	for (i = 0; i < prog_info.nr_map_ids; i++) {
-		map_fd = bpf_map_get_fd_by_id(map_ids[i]);
+		map_fd = bpf_map_get_fd_by_id_opts(map_ids[i], NULL);
 		if (map_fd < 0)
 			goto free_map_ids;
 
@@ -666,7 +666,7 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
-		fd = bpf_prog_get_fd_by_id(id);
+		fd = bpf_prog_get_fd_by_id_opts(id, NULL);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
-- 
2.25.1

[RFC][PATCH v3 14/15] bpftool: Adjust map permissions

[Roberto Sassu]

Request a read-only file descriptor for:
- btf subcommand: dump, show (build_btf_type_table for maps);
- do_build_table_cb(), to show the path of a pinned map;
- map search by name;
- iter subcommands: pin (maps);
- map subcommands: show_subset, show, dump, lookup, getnext and pin;
- prog subcommand: show (metadata);
- struct_ops subcommands: show and dump;
- retrieve fd of inner map for update of outer map.

Request a write-only file descriptor for:
- map subcommands: update, delete, event_pipe.

Other permissions requests remain unchanged.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/bpf/bpftool/btf.c           | 12 ++++++--
 tools/bpf/bpftool/common.c        | 30 +++++++++++++++++--
 tools/bpf/bpftool/iter.c          |  6 +++-
 tools/bpf/bpftool/map.c           | 48 +++++++++++++++++++++++++------
 tools/bpf/bpftool/map_perf_ring.c |  6 +++-
 tools/bpf/bpftool/prog.c          |  6 +++-
 tools/bpf/bpftool/struct_ops.c    | 32 +++++++++++++++++++--
 7 files changed, 121 insertions(+), 19 deletions(-)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 2cbc777f1520..4666a59d5fc6 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -566,6 +566,10 @@ static int do_dump(int argc, char **argv)
 	int fd = -1;
 	int err;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (!REQ_ARGS(2)) {
 		usage();
 		return -1;
@@ -580,7 +584,7 @@ static int do_dump(int argc, char **argv)
 			return -1;
 		}
 
-		fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
+		fd = map_parse_fd_and_info(&argc, &argv, &info, &len, &opts);
 		if (fd < 0)
 			return -1;
 
@@ -753,6 +757,10 @@ build_btf_type_table(struct hashmap *tab, enum bpf_obj_type type,
 	int err;
 	int fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	while (true) {
 		switch (type) {
 		case BPF_OBJ_PROG:
@@ -782,7 +790,7 @@ build_btf_type_table(struct hashmap *tab, enum bpf_obj_type type,
 			fd = bpf_prog_get_fd_by_id_opts(id, NULL);
 			break;
 		case BPF_OBJ_MAP:
-			fd = bpf_map_get_fd_by_id_opts(id, NULL);
+			fd = bpf_map_get_fd_by_id_opts(id, &opts);
 			break;
 		default:
 			err = -1;
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 8a2412fc4410..dd58054bcce7 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -28,6 +28,7 @@
 #include <bpf/hashmap.h>
 #include <bpf/libbpf.h> /* libbpf_num_possible_cpus */
 #include <bpf/btf.h>
+#include <bpf/libbpf_internal.h> /* OPTS_GET */
 
 #include "main.h"
 
@@ -303,7 +304,11 @@ int do_pin_any(int argc, char **argv,
 	int err;
 	int fd;
 
-	fd = get_fd(&argc, &argv, NULL);
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
+	fd = get_fd(&argc, &argv, &opts);
 	if (fd < 0)
 		return fd;
 
@@ -474,10 +479,14 @@ static int do_build_table_cb(const char *fpath, const struct stat *sb,
 	int fd, err = 0;
 	char *path;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (typeflag != FTW_F)
 		goto out_ret;
 
-	fd = open_obj_pinned(fpath, true, NULL);
+	fd = open_obj_pinned(fpath, true, &opts);
 	if (fd < 0)
 		goto out_ret;
 
@@ -886,6 +895,10 @@ static int map_fd_by_name(char *name, int **fds,
 	void *tmp;
 	int err;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, search_opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	while (true) {
 		struct bpf_map_info info = {};
 		__u32 len = sizeof(info);
@@ -899,7 +912,7 @@ static int map_fd_by_name(char *name, int **fds,
 			return nb_fds;
 		}
 
-		fd = bpf_map_get_fd_by_id_opts(id, opts);
+		fd = bpf_map_get_fd_by_id_opts(id, &search_opts);
 		if (fd < 0) {
 			p_err("can't get map by id (%u): %s",
 			      id, strerror(errno));
@@ -918,6 +931,17 @@ static int map_fd_by_name(char *name, int **fds,
 			continue;
 		}
 
+		if (OPTS_GET(opts, flags, 0) != BPF_F_RDONLY) {
+			close(fd);
+
+			fd = bpf_map_get_fd_by_id_opts(id, opts);
+			if (fd < 0) {
+				p_err("can't get map by id (%u): %s",
+				      id, strerror(errno));
+				goto err_close_fds;
+			}
+		}
+
 		if (nb_fds > 0) {
 			tmp = realloc(*fds, (nb_fds + 1) * sizeof(int));
 			if (!tmp) {
diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c
index 1412be9eb298..40b3f8fddd90 100644
--- a/tools/bpf/bpftool/iter.c
+++ b/tools/bpf/bpftool/iter.c
@@ -18,6 +18,10 @@ static int do_pin(int argc, char **argv)
 	struct bpf_link *link;
 	int err = -1, map_fd = -1;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (!REQ_ARGS(2))
 		usage();
 
@@ -34,7 +38,7 @@ static int do_pin(int argc, char **argv)
 				return -1;
 			}
 
-			map_fd = map_parse_fd(&argc, &argv, NULL);
+			map_fd = map_parse_fd(&argc, &argv, &opts);
 			if (map_fd < 0)
 				return -1;
 
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 103bd44cd851..e2936b0046ba 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -334,6 +334,10 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 		      void *key, void *value, __u32 key_size, __u32 value_size,
 		      __u32 *flags, __u32 **value_fd)
 {
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (!*argv) {
 		if (!key && !value)
 			return 0;
@@ -381,7 +385,7 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 				return -1;
 			}
 
-			fd = map_parse_fd(&argc, &argv, NULL);
+			fd = map_parse_fd(&argc, &argv, &opts);
 			if (fd < 0)
 				return -1;
 
@@ -629,12 +633,16 @@ static int do_show_subset(int argc, char **argv)
 	int nb_fds, i;
 	int err = -1;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	fds = malloc(sizeof(int));
 	if (!fds) {
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = map_parse_fds(&argc, &argv, &fds, NULL);
+	nb_fds = map_parse_fds(&argc, &argv, &fds, &opts);
 	if (nb_fds < 1)
 		goto exit_free;
 
@@ -673,6 +681,10 @@ static int do_show(int argc, char **argv)
 	int err;
 	int fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (show_pinned) {
 		map_table = hashmap__new(hash_fn_for_key_as_id,
 					 equal_fn_for_key_as_id, NULL);
@@ -702,7 +714,7 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
-		fd = bpf_map_get_fd_by_id_opts(id, NULL);
+		fd = bpf_map_get_fd_by_id_opts(id, &opts);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
@@ -902,6 +914,10 @@ static int do_dump(int argc, char **argv)
 	int *fds = NULL;
 	int err = -1;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (argc != 2)
 		usage();
 
@@ -910,7 +926,7 @@ static int do_dump(int argc, char **argv)
 		p_err("mem alloc failed");
 		return -1;
 	}
-	nb_fds = map_parse_fds(&argc, &argv, &fds, NULL);
+	nb_fds = map_parse_fds(&argc, &argv, &fds, &opts);
 	if (nb_fds < 1)
 		goto exit_free;
 
@@ -995,10 +1011,14 @@ static int do_update(int argc, char **argv)
 	void *key, *value;
 	int fd, err;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_WRONLY,
+	);
+
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, &opts);
 	if (fd < 0)
 		return -1;
 
@@ -1074,10 +1094,14 @@ static int do_lookup(int argc, char **argv)
 	int err;
 	int fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, &opts);
 	if (fd < 0)
 		return -1;
 
@@ -1125,10 +1149,14 @@ static int do_getnext(int argc, char **argv)
 	int err;
 	int fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, &opts);
 	if (fd < 0)
 		return -1;
 
@@ -1196,10 +1224,14 @@ static int do_delete(int argc, char **argv)
 	int err;
 	int fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_WRONLY,
+	);
+
 	if (argc < 2)
 		usage();
 
-	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, NULL);
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len, &opts);
 	if (fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c
index fa062db08c87..8bc513d6eb55 100644
--- a/tools/bpf/bpftool/map_perf_ring.c
+++ b/tools/bpf/bpftool/map_perf_ring.c
@@ -125,6 +125,10 @@ int do_event_pipe(int argc, char **argv)
 	};
 	struct bpf_map_info map_info = {};
 	LIBBPF_OPTS(perf_buffer_raw_opts, opts);
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, map_opts,
+		.flags = BPF_F_WRONLY,
+	);
+
 	struct event_pipe_ctx ctx = {
 		.all_cpus = true,
 		.cpu = -1,
@@ -136,7 +140,7 @@ int do_event_pipe(int argc, char **argv)
 
 	map_info_len = sizeof(map_info);
 	map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len,
-				       NULL);
+				       &map_opts);
 	if (map_fd < 0)
 		return -1;
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 648546342988..371a6510b2ed 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -227,6 +227,10 @@ static void *find_metadata(int prog_fd, struct bpf_map_info *map_info)
 	int ret;
 	__u32 i;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	memset(&prog_info, 0, sizeof(prog_info));
 	prog_info_len = sizeof(prog_info);
 	ret = bpf_obj_get_info_by_fd(prog_fd, &prog_info, &prog_info_len);
@@ -251,7 +255,7 @@ static void *find_metadata(int prog_fd, struct bpf_map_info *map_info)
 		goto free_map_ids;
 
 	for (i = 0; i < prog_info.nr_map_ids; i++) {
-		map_fd = bpf_map_get_fd_by_id_opts(map_ids[i], NULL);
+		map_fd = bpf_map_get_fd_by_id_opts(map_ids[i], &opts);
 		if (map_fd < 0)
 			goto free_map_ids;
 
diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
index 51667db3f55f..5a93f14e2b6a 100644
--- a/tools/bpf/bpftool/struct_ops.c
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -10,6 +10,7 @@
 #include <bpf/bpf.h>
 #include <bpf/btf.h>
 #include <bpf/libbpf.h>
+#include <bpf/libbpf_internal.h> /* OPTS_GET */
 
 #include "json_writer.h"
 #include "main.h"
@@ -136,6 +137,10 @@ static int get_next_struct_ops_map(const char *name, int *res_fd,
 	__u32 id = info->id;
 	int err, fd;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, search_opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	while (true) {
 		err = bpf_map_get_next_id(id, &id);
 		if (err) {
@@ -145,7 +150,7 @@ static int get_next_struct_ops_map(const char *name, int *res_fd,
 			return -1;
 		}
 
-		fd = bpf_map_get_fd_by_id_opts(id, opts);
+		fd = bpf_map_get_fd_by_id_opts(id, &search_opts);
 		if (fd < 0) {
 			if (errno == ENOENT)
 				continue;
@@ -163,6 +168,19 @@ static int get_next_struct_ops_map(const char *name, int *res_fd,
 
 		if (info->type == BPF_MAP_TYPE_STRUCT_OPS &&
 		    (!name || !strcmp(name, info->name))) {
+			if (OPTS_GET(opts, flags, 0) != BPF_F_RDONLY) {
+				close(fd);
+
+				fd = bpf_map_get_fd_by_id_opts(id, opts);
+				if (fd < 0) {
+					if (errno == ENOENT)
+						continue;
+					p_err("can't get map by id (%u): %s",
+					      id, strerror(errno));
+					return -1;
+				}
+			}
+
 			*res_fd = fd;
 			return 1;
 		}
@@ -340,6 +358,10 @@ static int do_show(int argc, char **argv)
 	const char *search_type = NULL, *search_term = NULL;
 	struct res res;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (argc && argc != 2)
 		usage();
 
@@ -349,7 +371,7 @@ static int do_show(int argc, char **argv)
 	}
 
 	res = do_work_on_struct_ops(search_type, search_term, __do_show,
-				    NULL, json_wtr, NULL);
+				    NULL, json_wtr, &opts);
 
 	return cmd_retval(&res, !!search_term);
 }
@@ -411,6 +433,10 @@ static int do_dump(int argc, char **argv)
 	struct btf_dumper d = {};
 	struct res res;
 
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts,
+		.flags = BPF_F_RDONLY,
+	);
+
 	if (argc && argc != 2)
 		usage();
 
@@ -438,7 +464,7 @@ static int do_dump(int argc, char **argv)
 	d.prog_id_as_func_ptr = true;
 
 	res = do_work_on_struct_ops(search_type, search_term, __do_dump, &d,
-				    wtr, NULL);
+				    wtr, &opts);
 
 	if (!json_output)
 		jsonw_destroy(&wtr);
-- 
2.25.1

[RFC][PATCH v3 15/15] selftests/bpf: Add map access tests

[Roberto Sassu]

Check the correctness of permission requests on two maps, one with
read-only access, and one with read-write access. Accesses are enforced
with a small eBPF program implementing the bpf_map security hook.

Ensure that read-like operations can be still executed on a read-only map,
unlike before where they were denied due to the requestor unnecessarily
specifying read-write permissions.

Also ensure that the read-write map can be still accessed when requesting
a write-like operation, unlike before where the search would stop at the
read-only map due to not having sufficient permissions.

Do the tests programmatically, with the new functions added to libbpf
accepting the opts parameter, and with the bpftool binary.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../bpf/prog_tests/map_check_access.c         | 186 ++++++++++++++++++
 .../bpf/progs/test_map_check_access.c         | 112 +++++++++++
 3 files changed, 300 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/map_check_access.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_map_check_access.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 8d59ec7f4c2d..a4f0b6f5c9f1 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -501,6 +501,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)			\
 	$(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
 	$(Q)$(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.o $$@
 	$(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/bootstrap/bpftool $(if $2,$2/)bpftool
+	$(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/bpftool $(if $2,$2/)bpftool_nobootstrap
 
 endef
 
@@ -595,7 +596,7 @@ $(OUTPUT)/bench: $(OUTPUT)/bench.o \
 
 EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
-	feature bpftool							\
+	feature bpftool bpftool_nobootstrap				\
 	$(addprefix $(OUTPUT)/,*.o *.skel.h *.lskel.h *.subskel.h	\
 			       no_alu32 bpf_gcc bpf_testmod.ko		\
 			       liburandom_read.so)
diff --git a/tools/testing/selftests/bpf/prog_tests/map_check_access.c b/tools/testing/selftests/bpf/prog_tests/map_check_access.c
new file mode 100644
index 000000000000..c2d801503a87
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/map_check_access.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ */
+
+#include <sys/stat.h>
+#include <test_progs.h>
+
+#include "test_map_check_access.skel.h"
+
+#define PINNED_MAP_PATH "/sys/fs/bpf/test_map_check_access_map"
+#define PINNED_ITER_PATH "/sys/fs/bpf/test_map_check_access_iter"
+#define BPFTOOL_PATH "./bpftool_nobootstrap"
+#define MAX_CMD_SIZE 1024
+
+enum check_types { CHECK_NONE, CHECK_PINNED, CHECK_METADATA, CHECK_PERF };
+
+struct bpftool_command {
+	char str[MAX_CMD_SIZE];
+	enum check_types check;
+	bool failure;
+};
+
+struct bpftool_command bpftool_commands[] = {
+	{ .str = BPFTOOL_PATH " map list" },
+	{ .str = BPFTOOL_PATH " map show name data_input" },
+	{ .str = BPFTOOL_PATH " map -f show pinned " PINNED_MAP_PATH,
+	  .check = CHECK_PINNED },
+	{ .str = "rm -f " PINNED_MAP_PATH },
+	{ .str = BPFTOOL_PATH " map dump name data_input" },
+	{ .str = BPFTOOL_PATH " map lookup name data_input key 0 0 0 0" },
+	{ .str = BPFTOOL_PATH
+	  " map update name data_input key 0 0 0 0 value 0 0 0 0 2> /dev/null",
+	  .failure = true },
+	{ .str = BPFTOOL_PATH
+	  " map update name data_input_mim key 0 0 0 0 value name data_input" },
+	{ .str = BPFTOOL_PATH
+	  " map update name data_input_w key 0 0 0 0 value 0 0 0 0" },
+	{ .str = BPFTOOL_PATH " iter pin test_map_check_access.o "
+		 PINNED_ITER_PATH " map name data_input" },
+	{ .str = "cat " PINNED_ITER_PATH },
+	{ .str = "rm -f " PINNED_ITER_PATH },
+	{ .str = BPFTOOL_PATH " prog show name check_access",
+	  .check = CHECK_METADATA },
+	{ .str = BPFTOOL_PATH " btf show" },
+	{ .str = BPFTOOL_PATH " btf dump map name data_input" },
+	{ .str = BPFTOOL_PATH " map pin name data_input " PINNED_MAP_PATH },
+	{ .str = BPFTOOL_PATH " struct_ops show name dummy_2" },
+	{ .str = BPFTOOL_PATH " struct_ops dump name dummy_2" },
+	{ .str = BPFTOOL_PATH " map event_pipe name data_input_perf",
+	  .check = CHECK_PERF },
+};
+
+static int _run_bpftool(struct bpftool_command *command)
+{
+	char output[1024] = { 0 };
+	FILE *fp;
+	int ret;
+
+	fp = popen(command->str, "r");
+	if (!fp)
+		return -errno;
+
+	fread(output, sizeof(output) - 1, sizeof(*output), fp);
+
+	ret = pclose(fp);
+	if (WEXITSTATUS(ret) && !command->failure)
+		return WEXITSTATUS(ret);
+
+	ret = 0;
+
+	switch (command->check) {
+	case CHECK_PINNED:
+		if (!strstr(output, PINNED_MAP_PATH))
+			ret = -ENOENT;
+		break;
+	case CHECK_METADATA:
+		if (!strstr(output, "test_var"))
+			ret = -ENOENT;
+		break;
+	case CHECK_PERF:
+		if (strncmp(output, "==", 2))
+			ret = -ENOENT;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+void test_map_check_access(void)
+{
+	struct test_map_check_access *skel;
+	struct bpf_map_info info_m = { 0 };
+	struct bpf_map *map;
+	__u32 len = sizeof(info_m);
+	int ret, zero = 0, fd, i;
+
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, opts_rdonly,
+		.flags = BPF_F_RDONLY,
+	);
+
+	skel = test_map_check_access__open();
+	if (!ASSERT_OK_PTR(skel, "test_map_check_access__open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.dump_bpf_hash_map, false);
+
+	ret = test_map_check_access__load(skel);
+	if (!ASSERT_OK(ret, "test_map_check_access__load"))
+		goto close_prog;
+
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_iter"))
+		goto close_prog;
+
+	ret = test_map_check_access__attach(skel);
+	if (!ASSERT_OK(ret, "test_map_check_access__attach"))
+		goto close_prog;
+
+	map = bpf_object__find_map_by_name(skel->obj, "data_input");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name"))
+		goto close_prog;
+
+	ret = bpf_obj_get_info_by_fd(bpf_map__fd(map), &info_m, &len);
+	if (!ASSERT_OK(ret, "bpf_obj_get_info_by_fd"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id(info_m.id);
+	if (!ASSERT_LT(fd, 0, "bpf_map_get_fd_by_id"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id_opts(info_m.id, NULL);
+	if (!ASSERT_LT(fd, 0, "bpf_map_get_fd_by_id_opts"))
+		goto close_prog;
+
+	fd = bpf_map_get_fd_by_id_opts(info_m.id, &opts_rdonly);
+	if (!ASSERT_GE(fd, 0, "bpf_map_get_fd_by_id_opts"))
+		goto close_prog;
+
+	ret = bpf_map_lookup_elem(fd, &zero, &len);
+	if (!ASSERT_OK(ret, "bpf_map_lookup_elem")) {
+		close(fd);
+		goto close_prog;
+	}
+
+	ret = bpf_map_update_elem(fd, &zero, &len, BPF_ANY);
+
+	close(fd);
+
+	if (!ASSERT_LT(ret, 0, "bpf_map_update_elem"))
+		goto close_prog;
+
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &len, BPF_ANY);
+	if (!ASSERT_OK(ret, "bpf_map_update_elem"))
+		goto close_prog;
+
+	ret = bpf_map__pin(map, PINNED_MAP_PATH);
+	if (!ASSERT_OK(ret, "bpf_map__pin"))
+		goto close_prog;
+
+	fd = bpf_obj_get_opts(PINNED_MAP_PATH, &opts_rdonly);
+	if (!ASSERT_GE(fd, 0, "bpf_obj_get_opts"))
+		goto close_prog;
+
+	close(fd);
+
+	fd = bpf_obj_get_opts(PINNED_MAP_PATH, NULL);
+	if (!ASSERT_LT(fd, 0, "bpf_obj_get_opts")) {
+		close(fd);
+		goto close_prog;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(bpftool_commands); i++) {
+		ret = _run_bpftool(&bpftool_commands[i]);
+		if (!ASSERT_OK(ret, bpftool_commands[i].str))
+			goto close_prog;
+	}
+
+close_prog:
+	test_map_check_access__destroy(skel);
+	unlink(PINNED_MAP_PATH);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_map_check_access.c b/tools/testing/selftests/bpf/progs/test_map_check_access.c
new file mode 100644
index 000000000000..6b8f0bf8f77e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_map_check_access.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2021. Huawei Technologies Co., Ltd
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* From include/linux/mm.h. */
+#define FMODE_WRITE	0x2
+
+const char bpf_metadata_test_var[] SEC(".rodata") = "test_var";
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u32);
+} data_input SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u32);
+} data_input_w SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
+	__uint(max_entries, 1);
+	__uint(map_flags, 0);
+	__type(key, __u32);
+	__type(value, __u32);
+	__array(values, struct {
+		__uint(type, BPF_MAP_TYPE_ARRAY);
+		__uint(max_entries, 1);
+		__type(key, int);
+		__type(value, int);
+	});
+} data_input_mim SEC(".maps") = {
+	.values = { (void *)&data_input },
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
+	__type(key, int);
+	__type(value, int);
+} data_input_perf SEC(".maps");
+
+char _license[] SEC("license") = "GPL";
+
+SEC("iter/bpf_map_elem")
+int dump_bpf_hash_map(struct bpf_iter__bpf_map_elem *ctx)
+{
+	struct seq_file *seq = ctx->meta->seq;
+	struct bpf_map *map = ctx->map;
+	u32 *key = ctx->key;
+	u32 *val = ctx->value;
+
+	if (key == (void *)0 || val == (void *)0)
+		return 0;
+
+	BPF_SEQ_PRINTF(seq, "%d: (%x) (%llx)\n", map->id, *key, *val);
+	return 0;
+}
+
+SEC("lsm/bpf_map")
+int BPF_PROG(check_access, struct bpf_map *map, fmode_t fmode)
+{
+	if (map != (struct bpf_map *)&data_input)
+		return 0;
+
+	if (fmode & FMODE_WRITE)
+		return -EACCES;
+
+	return 0;
+}
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1, struct bpf_dummy_ops_state *state)
+{
+	return 0;
+}
+
+SEC("struct_ops/test_2")
+int BPF_PROG(test_2, struct bpf_dummy_ops_state *state, int a1,
+	     unsigned short a2, char a3, unsigned long a4)
+{
+	return 0;
+}
+
+SEC(".struct_ops")
+struct bpf_dummy_ops dummy_2 = {
+	.test_1 = (void *)test_1,
+	.test_2 = (void *)test_2,
+};
+
+SEC("tp/raw_syscalls/sys_enter")
+int handle_sys_enter(void *ctx)
+{
+	int cpu = bpf_get_smp_processor_id();
+
+	bpf_perf_event_output(ctx, &data_input_perf, BPF_F_CURRENT_CPU,
+			      &cpu, sizeof(cpu));
+	return 0;
+}
-- 
2.25.1

Re: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Alexei Starovoitov]

On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> The bpf() system call validates the bpf_attr structure received as
> argument, and considers data until the last field, defined for each
> operation. The remaing space must be filled with zeros.
> 
> Currently, for bpf_*_get_fd_by_id() functions except bpf_map_get_fd_by_id()
> the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space
> will result in bpf() rejecting the argument.

The kernel is doing the right thing. It should not ignore fields.

Re: [RFC][PATCH v3 07/15] libbpf: Introduce bpf_obj_get_opts()

[Stanislav Fomichev]

On Fri, Jul 22, 2022 at 10:20 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> Introduce bpf_obj_get_opts(), to let the caller pass the needed permissions
> for the operation. Keep the existing bpf_obj_get() to request read-write
> permissions.
>
> bpf_obj_get() allows the caller to get a file descriptor from a pinned
> object with the provided pathname. Specifying permissions has only effect
> on maps (for links, the permission must be always read-write).
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/lib/bpf/bpf.c      | 12 +++++++++++-
>  tools/lib/bpf/bpf.h      |  2 ++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 5f2785a4c358..0df088890864 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -577,18 +577,28 @@ int bpf_obj_pin(int fd, const char *pathname)
>         return libbpf_err_errno(ret);
>  }
>
> -int bpf_obj_get(const char *pathname)
> +int bpf_obj_get_opts(const char *pathname,
> +                    const struct bpf_get_fd_opts *opts)

I'm still not sure whether it's a good idea to mix get_fd with
obj_get/pin operations? [1] seems more clear.
It just so happens that (differently named) flags in BPF_OBJ_GET and
BPF_XXX_GET_FD_BY_ID align, but maybe we shouldn't depend on it?

Also, it seems only bpf_map_get_fd_by_id currently accepts flags? So
this sharing makes even more sense?

[1] https://lore.kernel.org/bpf/20220719194028.4180569-1-jevburton.kernel@gmail.com/





>  {
>         union bpf_attr attr;
>         int fd;
>
> +       if (!OPTS_VALID(opts, bpf_get_fd_opts))
> +               return libbpf_err(-EINVAL);
> +
>         memset(&attr, 0, sizeof(attr));
>         attr.pathname = ptr_to_u64((void *)pathname);
> +       attr.file_flags = OPTS_GET(opts, flags, 0);
>
>         fd = sys_bpf_fd(BPF_OBJ_GET, &attr, sizeof(attr));
>         return libbpf_err_errno(fd);
>  }
>
> +int bpf_obj_get(const char *pathname)
> +{
> +       return bpf_obj_get_opts(pathname, NULL);
> +}
> +
>  int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
>                     unsigned int flags)
>  {
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index b75fd9d37bad..e0c5018cc131 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -279,6 +279,8 @@ struct bpf_get_fd_opts {
>  };
>  #define bpf_get_fd_opts__last_field flags
>
> +LIBBPF_API int bpf_obj_get_opts(const char *pathname,
> +                               const struct bpf_get_fd_opts *opts);
>  LIBBPF_API int bpf_obj_get(const char *pathname);
>
>  struct bpf_prog_attach_opts {
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index dba97d2ef8a9..55516b19e22f 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -368,4 +368,5 @@ LIBBPF_1.0.0 {
>                 bpf_map_get_fd_by_id_opts;
>                 bpf_btf_get_fd_by_id_opts;
>                 bpf_link_get_fd_by_id_opts;
> +               bpf_obj_get_opts;
>  };
> --
> 2.25.1
>

Re: [RFC][PATCH v3 07/15] libbpf: Introduce bpf_obj_get_opts()

[Alexei Starovoitov]

On Fri, Jul 22, 2022 at 10:58 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Fri, Jul 22, 2022 at 10:20 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
> >
> > Introduce bpf_obj_get_opts(), to let the caller pass the needed permissions
> > for the operation. Keep the existing bpf_obj_get() to request read-write
> > permissions.
> >
> > bpf_obj_get() allows the caller to get a file descriptor from a pinned
> > object with the provided pathname. Specifying permissions has only effect
> > on maps (for links, the permission must be always read-write).
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  tools/lib/bpf/bpf.c      | 12 +++++++++++-
> >  tools/lib/bpf/bpf.h      |  2 ++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 5f2785a4c358..0df088890864 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -577,18 +577,28 @@ int bpf_obj_pin(int fd, const char *pathname)
> >         return libbpf_err_errno(ret);
> >  }
> >
> > -int bpf_obj_get(const char *pathname)
> > +int bpf_obj_get_opts(const char *pathname,
> > +                    const struct bpf_get_fd_opts *opts)
>
> I'm still not sure whether it's a good idea to mix get_fd with
> obj_get/pin operations? [1] seems more clear.

+1

> It just so happens that (differently named) flags in BPF_OBJ_GET and
> BPF_XXX_GET_FD_BY_ID align, but maybe we shouldn't depend on it?
>
> Also, it seems only bpf_map_get_fd_by_id currently accepts flags? So
> this sharing makes even more sense?

+1

Roberto, the patch set is broken in many ways.

RE: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Roberto Sassu]

> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Friday, July 22, 2022 7:55 PM
> On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> > The bpf() system call validates the bpf_attr structure received as
> > argument, and considers data until the last field, defined for each
> > operation. The remaing space must be filled with zeros.
> >
> > Currently, for bpf_*_get_fd_by_id() functions except bpf_map_get_fd_by_id()
> > the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space
> > will result in bpf() rejecting the argument.
> 
> The kernel is doing the right thing. It should not ignore fields.

Exactly. As Andrii requested to add opts to all bpf_*_get_fd_by_id()
functions, the last field in the kernel needs to be updated accordingly.

Roberto

RE: [RFC][PATCH v3 07/15] libbpf: Introduce bpf_obj_get_opts()

[Roberto Sassu]

> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Friday, July 22, 2022 8:02 PM
> On Fri, Jul 22, 2022 at 10:58 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > On Fri, Jul 22, 2022 at 10:20 AM Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> > >
> > > Introduce bpf_obj_get_opts(), to let the caller pass the needed permissions
> > > for the operation. Keep the existing bpf_obj_get() to request read-write
> > > permissions.
> > >
> > > bpf_obj_get() allows the caller to get a file descriptor from a pinned
> > > object with the provided pathname. Specifying permissions has only effect
> > > on maps (for links, the permission must be always read-write).
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > ---
> > >  tools/lib/bpf/bpf.c      | 12 +++++++++++-
> > >  tools/lib/bpf/bpf.h      |  2 ++
> > >  tools/lib/bpf/libbpf.map |  1 +
> > >  3 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > > index 5f2785a4c358..0df088890864 100644
> > > --- a/tools/lib/bpf/bpf.c
> > > +++ b/tools/lib/bpf/bpf.c
> > > @@ -577,18 +577,28 @@ int bpf_obj_pin(int fd, const char *pathname)
> > >         return libbpf_err_errno(ret);
> > >  }
> > >
> > > -int bpf_obj_get(const char *pathname)
> > > +int bpf_obj_get_opts(const char *pathname,
> > > +                    const struct bpf_get_fd_opts *opts)
> >
> > I'm still not sure whether it's a good idea to mix get_fd with
> > obj_get/pin operations? [1] seems more clear.
> 
> +1

I think so. Both types of functions are accessing the same object,
just in a different way: one by ID, and another by path.

Consider the case I mentioned, map_parse_fds() in bpftool. It calls
both type of functions. What opts a caller of this function should
provide, if they are different?

> > It just so happens that (differently named) flags in BPF_OBJ_GET and
> > BPF_XXX_GET_FD_BY_ID align, but maybe we shouldn't depend on it?
> >
> > Also, it seems only bpf_map_get_fd_by_id currently accepts flags? So
> > this sharing makes even more sense?

As I mentioned in another email, Andrii requested me in v2 to add
opts to all bpf_*_get_fd_by_id() functions.

> +1
> 
> Roberto, the patch set is broken in many ways.

Could you please explain?

Thanks

Roberto

Re: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Andrii Nakryiko]

On Mon, Jul 25, 2022 at 12:10 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > Sent: Friday, July 22, 2022 7:55 PM
> > On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> > > The bpf() system call validates the bpf_attr structure received as
> > > argument, and considers data until the last field, defined for each
> > > operation. The remaing space must be filled with zeros.
> > >
> > > Currently, for bpf_*_get_fd_by_id() functions except bpf_map_get_fd_by_id()
> > > the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space
> > > will result in bpf() rejecting the argument.
> >
> > The kernel is doing the right thing. It should not ignore fields.
>
> Exactly. As Andrii requested to add opts to all bpf_*_get_fd_by_id()
> functions, the last field in the kernel needs to be updated accordingly.
>

It's been a while ago so details are hazy. But the idea was that if we
add _opts variant for bpf_map_get_fd_by_id() for interface consistency
all the other bpf_*_get_fd_by_id() probably should get _opts variant
and use the same opts struct. Right now kernel doesn't support
specifying flags for non-maps and that's fine. I agree with Alexei
that kernel shouldn't just ignore unrecognized field silently.

I think we still can add _opts() for all APIs, but user will need to
know that non-map variants expect 0 as flags. For now. If we
eventually add ability to specify flags for, say, links, then existing
API will just work. One can see how this get_fd_by_id() can use
read-only flags to return FDs that only support read-only operations
on objects (e.g., fetching link info for links, dumping prog
instructions for programs), but not modification operations (e.g.,
updating prog for links, or whatever write operation could be for
programs).

So I don't think there is contradiction here. We might choose to add
bpf_map_get_fd_by_id_opts() only, but we probably still should use
common struct name as if all bpf_*_get_fd_by_id_opts() exist.


> Roberto

Re: [RFC][PATCH v3 03/15] libbpf: Introduce bpf_prog_get_fd_by_id_opts()

[Andrii Nakryiko]

On Fri, Jul 22, 2022 at 10:19 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> Define a new data structure called bpf_get_fd_opts, with the member flags,
> to be used by callers of the _opts variants of bpf_*_get_fd_by_id() and
> bpf_obj_get() to specify the permissions needed for the operations on the
> obtained file descriptor.
>
> Define only one data structure for all variants, as the open_flags field in
> bpf_attr for bpf_*_get_fd_by_id_opts() and file_flags for
> bpf_obj_get_opts() have the same meaning. Also, it would avoid the

as I replied elsewhere, bpf_obj_get_opts() is a different bpf()
command, so it makes sense to use a separate opts struct for it

> confusion when the same bpftool function calls both
> bpf_*_get_fd_by_id_opts() and bpf_obj_get_opts() (e.g. map_parse_fds()).
>
> Then, introduce the new feature FEAT_GET_FD_BY_ID_OPEN_FLAGS to determine
> whether or not the kernel supports setting open_flags for
> bpf_*_get_fd_by_id() functions (except for bpf_map_get_fd_by_id(), which
> already can get it). If the feature is not detected, the _opts variants
> ignore flags in the bpf_get_fd_opts structure and leave open_flags to zero.
>
> Finally, introduce bpf_prog_get_fd_by_id_opts(), to let the caller pass the
> newly introduced data structure. Keep the existing bpf_prog_get_fd_by_id(),
> and call bpf_prog_get_fd_by_id_opts() with NULL as opts argument.
>
> Currently, setting permissions in the data structure has no effect, as the
> kernel does not evaluate them. The new variant has been introduced anyway
> for symmetry with bpf_map_get_fd_by_id_opts(). Evaluating permissions could
> be done in future kernel versions.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/lib/bpf/bpf.c             | 37 ++++++++++++++++++++++++++++++++-
>  tools/lib/bpf/bpf.h             | 11 ++++++++++
>  tools/lib/bpf/libbpf.c          |  4 ++++
>  tools/lib/bpf/libbpf.map        |  1 +
>  tools/lib/bpf/libbpf_internal.h |  3 +++
>  5 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 5eb0df90eb2b..9014a61bca83 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -910,18 +910,53 @@ int bpf_link_get_next_id(__u32 start_id, __u32 *next_id)
>         return bpf_obj_get_next_id(start_id, next_id, BPF_LINK_GET_NEXT_ID);
>  }
>
> -int bpf_prog_get_fd_by_id(__u32 id)
> +static int
> +bpf_prog_get_fd_by_id_opts_check(__u32 id, const struct bpf_get_fd_opts *opts,
> +                                bool check_support)
>  {
>         union bpf_attr attr;
>         int fd;
>
> +       if (!OPTS_VALID(opts, bpf_get_fd_opts))
> +               return libbpf_err(-EINVAL);
> +
>         memset(&attr, 0, sizeof(attr));
>         attr.prog_id = id;
> +       if (!check_support ||
> +           kernel_supports(NULL, FEAT_GET_FD_BY_ID_OPEN_FLAGS))
> +               attr.open_flags = OPTS_GET(opts, flags, 0);

low-level APIs from libbpf/bpf.h, as a general rule, don't do such
feature detection and don't ignore user-provided flags just because
kernel doesn't support them. So I think this is wrong approach, just
pass through user flags and user will have to make sure to pass
correct value (potentially just zero, or just pass NULL as opts).

>
>         fd = sys_bpf_fd(BPF_PROG_GET_FD_BY_ID, &attr, sizeof(attr));
>         return libbpf_err_errno(fd);
>  }
>

[...]

Re: [RFC][PATCH v3 04/15] libbpf: Introduce bpf_map_get_fd_by_id_opts()

[Andrii Nakryiko]

On Fri, Jul 22, 2022 at 10:19 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> Introduce bpf_map_get_fd_by_id_opts(), to let the caller pass a
> bpf_get_fd_opts structure with flags set to the permissions necessary to
> perform the operations on the obtained file descriptor.
>
> Don't check FEAT_GET_FD_BY_ID_OPEN_FLAGS, as current kernels already take
> open_flags as last bpf_attr field for this request.
>
> Keep the existing bpf_map_get_fd_by_id(), and call
> bpf_map_get_fd_by_id_opts() with NULL as opts argument, to request
> read-write permissions.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/lib/bpf/bpf.c      | 12 +++++++++++-
>  tools/lib/bpf/bpf.h      |  2 ++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 9014a61bca83..4b574ad046f3 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -957,18 +957,28 @@ int bpf_prog_get_fd_by_id(__u32 id)
>         return bpf_prog_get_fd_by_id_opts(id, NULL);
>  }
>
> -int bpf_map_get_fd_by_id(__u32 id)
> +int bpf_map_get_fd_by_id_opts(__u32 id,
> +                             const struct bpf_get_fd_opts *opts)
>  {
>         union bpf_attr attr;
>         int fd;
>
> +       if (!OPTS_VALID(opts, bpf_get_fd_opts))
> +               return libbpf_err(-EINVAL);
> +
>         memset(&attr, 0, sizeof(attr));
>         attr.map_id = id;
> +       attr.open_flags = OPTS_GET(opts, flags, 0);
>
>         fd = sys_bpf_fd(BPF_MAP_GET_FD_BY_ID, &attr, sizeof(attr));
>         return libbpf_err_errno(fd);
>  }
>
> +int bpf_map_get_fd_by_id(__u32 id)
> +{
> +       return bpf_map_get_fd_by_id_opts(id, NULL);
> +}
> +
>  int bpf_btf_get_fd_by_id(__u32 id)
>  {
>         union bpf_attr attr;
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index bc241343a0f9..d4b84d3f7e16 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -366,6 +366,8 @@ LIBBPF_API int bpf_link_get_next_id(__u32 start_id, __u32 *next_id);
>  LIBBPF_API int bpf_prog_get_fd_by_id_opts(__u32 id,
>                                           const struct bpf_get_fd_opts *opts);
>  LIBBPF_API int bpf_prog_get_fd_by_id(__u32 id);
> +LIBBPF_API int bpf_map_get_fd_by_id_opts(__u32 id,
> +                                        const struct bpf_get_fd_opts *opts);
>  LIBBPF_API int bpf_map_get_fd_by_id(__u32 id);
>  LIBBPF_API int bpf_btf_get_fd_by_id(__u32 id);
>  LIBBPF_API int bpf_link_get_fd_by_id(__u32 id);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index ab818612a585..83dc18b5e5cf 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -365,4 +365,5 @@ LIBBPF_1.0.0 {
>                 libbpf_bpf_prog_type_str;
>                 perf_buffer__buffer;
>                 bpf_prog_get_fd_by_id_opts;
> +               bpf_map_get_fd_by_id_opts;

keep in mind that this list is alphabetically sorted

>  };
> --
> 2.25.1
>

RE: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Roberto Sassu]

> From: Andrii Nakryiko [mailto:andrii.nakryiko@gmail.com]
> Sent: Friday, July 29, 2022 8:49 PM
> On Mon, Jul 25, 2022 at 12:10 AM Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> >
> > > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > > Sent: Friday, July 22, 2022 7:55 PM
> > > On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> > > > The bpf() system call validates the bpf_attr structure received as
> > > > argument, and considers data until the last field, defined for each
> > > > operation. The remaing space must be filled with zeros.
> > > >
> > > > Currently, for bpf_*_get_fd_by_id() functions except
> bpf_map_get_fd_by_id()
> > > > the last field is *_id. Setting open_flags to BPF_F_RDONLY from user space
> > > > will result in bpf() rejecting the argument.
> > >
> > > The kernel is doing the right thing. It should not ignore fields.
> >
> > Exactly. As Andrii requested to add opts to all bpf_*_get_fd_by_id()
> > functions, the last field in the kernel needs to be updated accordingly.
> >
> 
> It's been a while ago so details are hazy. But the idea was that if we
> add _opts variant for bpf_map_get_fd_by_id() for interface consistency
> all the other bpf_*_get_fd_by_id() probably should get _opts variant
> and use the same opts struct. Right now kernel doesn't support
> specifying flags for non-maps and that's fine. I agree with Alexei
> that kernel shouldn't just ignore unrecognized field silently.
> 
> I think we still can add _opts() for all APIs, but user will need to
> know that non-map variants expect 0 as flags. For now. If we
> eventually add ability to specify flags for, say, links, then existing
> API will just work. One can see how this get_fd_by_id() can use
> read-only flags to return FDs that only support read-only operations
> on objects (e.g., fetching link info for links, dumping prog
> instructions for programs), but not modification operations (e.g.,
> updating prog for links, or whatever write operation could be for
> programs).
> 
> So I don't think there is contradiction here. We might choose to add
> bpf_map_get_fd_by_id_opts() only, but we probably still should use
> common struct name as if all bpf_*_get_fd_by_id_opts() exist.

Ok, understood.

Thanks

Roberto

Re: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Roberto Sassu]

On Mon, 2022-08-01 at 10:33 +0000, Roberto Sassu wrote:
> > From: Andrii Nakryiko [mailto:andrii.nakryiko@gmail.com]
> > Sent: Friday, July 29, 2022 8:49 PM
> > On Mon, Jul 25, 2022 at 12:10 AM Roberto Sassu <
> > roberto.sassu@huawei.com>
> > wrote:
> > > > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > > > Sent: Friday, July 22, 2022 7:55 PM
> > > > On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> > > > > The bpf() system call validates the bpf_attr structure
> > > > > received as
> > > > > argument, and considers data until the last field, defined
> > > > > for each
> > > > > operation. The remaing space must be filled with zeros.
> > > > > 
> > > > > Currently, for bpf_*_get_fd_by_id() functions except
> > bpf_map_get_fd_by_id()
> > > > > the last field is *_id. Setting open_flags to BPF_F_RDONLY
> > > > > from user space
> > > > > will result in bpf() rejecting the argument.
> > > > 
> > > > The kernel is doing the right thing. It should not ignore
> > > > fields.
> > > 
> > > Exactly. As Andrii requested to add opts to all
> > > bpf_*_get_fd_by_id()
> > > functions, the last field in the kernel needs to be updated
> > > accordingly.
> > > 
> > 
> > It's been a while ago so details are hazy. But the idea was that if
> > we
> > add _opts variant for bpf_map_get_fd_by_id() for interface
> > consistency
> > all the other bpf_*_get_fd_by_id() probably should get _opts
> > variant
> > and use the same opts struct. Right now kernel doesn't support
> > specifying flags for non-maps and that's fine. I agree with Alexei
> > that kernel shouldn't just ignore unrecognized field silently.
> > 
> > I think we still can add _opts() for all APIs, but user will need
> > to
> > know that non-map variants expect 0 as flags. For now. If we
> > eventually add ability to specify flags for, say, links, then
> > existing
> > API will just work. One can see how this get_fd_by_id() can use
> > read-only flags to return FDs that only support read-only
> > operations
> > on objects (e.g., fetching link info for links, dumping prog
> > instructions for programs), but not modification operations (e.g.,
> > updating prog for links, or whatever write operation could be for
> > programs).
> > 
> > So I don't think there is contradiction here. We might choose to
> > add
> > bpf_map_get_fd_by_id_opts() only, but we probably still should use
> > common struct name as if all bpf_*_get_fd_by_id_opts() exist.
> 
> Ok, understood.

Hi Andrii

I'm about to send v4 with the suggestions you made.

Since now libbpf v1 has been released, how it works for new patches? It
seems there is not a new section in libbpf.map (kernel) new API
functions should be added to.

Also, I'm using a custom step in the CI:

https://github.com/robertosassu/libbpf-ci/commit/7743eb92f81f95355571c07e5b7082a9a2b0bfe0

Do you want me to create a new PR before sending the patch set?

Thanks

Roberto

Re: [RFC][PATCH v3 02/15] bpf: Set open_flags as last bpf_attr field for bpf_*_get_fd_by_id() funcs

[Andrii Nakryiko]

On Thu, Aug 25, 2022 at 5:21 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> On Mon, 2022-08-01 at 10:33 +0000, Roberto Sassu wrote:
> > > From: Andrii Nakryiko [mailto:andrii.nakryiko@gmail.com]
> > > Sent: Friday, July 29, 2022 8:49 PM
> > > On Mon, Jul 25, 2022 at 12:10 AM Roberto Sassu <
> > > roberto.sassu@huawei.com>
> > > wrote:
> > > > > From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> > > > > Sent: Friday, July 22, 2022 7:55 PM
> > > > > On Fri, Jul 22, 2022 at 07:18:23PM +0200, Roberto Sassu wrote:
> > > > > > The bpf() system call validates the bpf_attr structure
> > > > > > received as
> > > > > > argument, and considers data until the last field, defined
> > > > > > for each
> > > > > > operation. The remaing space must be filled with zeros.
> > > > > >
> > > > > > Currently, for bpf_*_get_fd_by_id() functions except
> > > bpf_map_get_fd_by_id()
> > > > > > the last field is *_id. Setting open_flags to BPF_F_RDONLY
> > > > > > from user space
> > > > > > will result in bpf() rejecting the argument.
> > > > >
> > > > > The kernel is doing the right thing. It should not ignore
> > > > > fields.
> > > >
> > > > Exactly. As Andrii requested to add opts to all
> > > > bpf_*_get_fd_by_id()
> > > > functions, the last field in the kernel needs to be updated
> > > > accordingly.
> > > >
> > >
> > > It's been a while ago so details are hazy. But the idea was that if
> > > we
> > > add _opts variant for bpf_map_get_fd_by_id() for interface
> > > consistency
> > > all the other bpf_*_get_fd_by_id() probably should get _opts
> > > variant
> > > and use the same opts struct. Right now kernel doesn't support
> > > specifying flags for non-maps and that's fine. I agree with Alexei
> > > that kernel shouldn't just ignore unrecognized field silently.
> > >
> > > I think we still can add _opts() for all APIs, but user will need
> > > to
> > > know that non-map variants expect 0 as flags. For now. If we
> > > eventually add ability to specify flags for, say, links, then
> > > existing
> > > API will just work. One can see how this get_fd_by_id() can use
> > > read-only flags to return FDs that only support read-only
> > > operations
> > > on objects (e.g., fetching link info for links, dumping prog
> > > instructions for programs), but not modification operations (e.g.,
> > > updating prog for links, or whatever write operation could be for
> > > programs).
> > >
> > > So I don't think there is contradiction here. We might choose to
> > > add
> > > bpf_map_get_fd_by_id_opts() only, but we probably still should use
> > > common struct name as if all bpf_*_get_fd_by_id_opts() exist.
> >
> > Ok, understood.
>
> Hi Andrii
>
> I'm about to send v4 with the suggestions you made.
>
> Since now libbpf v1 has been released, how it works for new patches? It
> seems there is not a new section in libbpf.map (kernel) new API
> functions should be added to.

yes, we need to add LIBBPF_1.1.0 now. I might send a small patch today
to do that, if not, feel free to add it in your patch set. Whoever
lands first wins (unfortunately even if I add an empty section, first
feature adding a new function to libbpf API would need to add
"global:" and thus conflict with any other patch set adding new API).

>
> Also, I'm using a custom step in the CI:
>
> https://github.com/robertosassu/libbpf-ci/commit/7743eb92f81f95355571c07e5b7082a9a2b0bfe0
>
> Do you want me to create a new PR before sending the patch set?
>
> Thanks
>
> Roberto
>