[RESEND][PATCH 0/6] Add _opts variant for bpf_*_get_fd_by_id()

[RESEND][PATCH 0/6] Add _opts variant for bpf_*_get_fd_by_id()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

NOTE: resending with libbpf_get_fd_opts test added to deny list for s390x.

Add the _opts variant for bpf_*_get_fd_by_id() functions, to be able to
pass to the kernel more options, when requesting a fd of an eBPF object to
the kernel.

Pass the options through a newly introduced structure, bpf_get_fd_opts,
which currently contains open_flags (the other two members are for
compatibility and for padding).

open_flags allows the caller to request specific permissions to access a
map (e.g. read-only). This is useful for example in the situation where a
map is write-protected.

Besides patches 2-6, which introduce the new variants and the data
structure, patch 1 fixes the LIBBPF_1.0.0 declaration in libbpf.map.

Roberto Sassu (6):
  libbpf: Fix LIBBPF_1.0.0 declaration in libbpf.map
  libbpf: Define bpf_get_fd_opts and introduce
    bpf_map_get_fd_by_id_opts()
  libbpf: Introduce bpf_prog_get_fd_by_id_opts()
  libbpf: Introduce bpf_btf_get_fd_by_id_opts()
  libbpf: Introduce bpf_link_get_fd_by_id_opts()
  selftests/bpf: Add tests for _opts variants of bpf_*_get_fd_by_id()

 tools/lib/bpf/bpf.c                           | 47 +++++++++-
 tools/lib/bpf/bpf.h                           | 16 ++++
 tools/lib/bpf/libbpf.map                      |  6 +-
 tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
 .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
 .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
 6 files changed, 189 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c

-- 
2.25.1

[RESEND][PATCH 1/6] libbpf: Fix LIBBPF_1.0.0 declaration in libbpf.map

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Add the missing LIBBPF_0.8.0 at the end of the LIBBPF_1.0.0 declaration,
similarly to other version declarations.

Cc: stable@vger.kernel.org # 5.19.x
Fixes: e2371b1632b1c ("libbpf: start 1.0 development cycle")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/lib/bpf/libbpf.map | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index c1d6aa7c82b6..04fab9f1fdd7 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -367,7 +367,7 @@ LIBBPF_1.0.0 {
 		libbpf_bpf_map_type_str;
 		libbpf_bpf_prog_type_str;
 		perf_buffer__buffer;
-};
+} LIBBPF_0.8.0;
 
 LIBBPF_1.1.0 {
 	global:
-- 
2.25.1

Re: [RESEND][PATCH 1/6] libbpf: Fix LIBBPF_1.0.0 declaration in libbpf.map

[Andrii Nakryiko]

On Tue, Oct 4, 2022 at 6:18 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Add the missing LIBBPF_0.8.0 at the end of the LIBBPF_1.0.0 declaration,
> similarly to other version declarations.
>
> Cc: stable@vger.kernel.org # 5.19.x

there is no need to backport this, libbpf has its own release cycle
and it is released from github mirror. And also this doesn't have any
effect, this inheritance between versions in .map file is mostly for
humans, according [0] (and I checked in practice that it doesn't
change anything for libbpf).

So it's good to fix, but no need to bother maintainers to backport it
to stable branches.

  [0] https://www.akkadia.org/drepper/dsohowto.pdf

> Fixes: e2371b1632b1c ("libbpf: start 1.0 development cycle")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/lib/bpf/libbpf.map | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index c1d6aa7c82b6..04fab9f1fdd7 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -367,7 +367,7 @@ LIBBPF_1.0.0 {
>                 libbpf_bpf_map_type_str;
>                 libbpf_bpf_prog_type_str;
>                 perf_buffer__buffer;
> -};
> +} LIBBPF_0.8.0;
>
>  LIBBPF_1.1.0 {
>         global:
> --
> 2.25.1
>

[RESEND][PATCH 2/6] libbpf: Define bpf_get_fd_opts and introduce bpf_map_get_fd_by_id_opts()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

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

Also, introduce bpf_map_get_fd_by_id_opts(), to let the caller pass a
bpf_get_fd_opts structure.

Finally, 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 (current behavior).

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

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 1d49a0352836..4b03063edf1d 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -948,19 +948,29 @@ int bpf_prog_get_fd_by_id(__u32 id)
 	return libbpf_err_errno(fd);
 }
 
-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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, attr_sz);
 	attr.map_id = id;
+	attr.open_flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_MAP_GET_FD_BY_ID, &attr, attr_sz);
 	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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 9c50beabdd14..c60eda3c88ad 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -365,7 +365,17 @@ 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);
+
+struct bpf_get_fd_opts {
+	size_t sz; /* size of this struct for forward/backward compatibility */
+	__u32 open_flags; /* permissions requested for the operation on fd */
+	size_t :0;
+};
+#define bpf_get_fd_opts__last_field open_flags
+
 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 04fab9f1fdd7..2e665b21d84f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -371,6 +371,7 @@ LIBBPF_1.0.0 {
 
 LIBBPF_1.1.0 {
 	global:
+		bpf_map_get_fd_by_id_opts;
 		user_ring_buffer__discard;
 		user_ring_buffer__free;
 		user_ring_buffer__new;
-- 
2.25.1

Re: [RESEND][PATCH 2/6] libbpf: Define bpf_get_fd_opts and introduce bpf_map_get_fd_by_id_opts()

[Andrii Nakryiko]

On Tue, Oct 4, 2022 at 6:18 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Define a new data structure called bpf_get_fd_opts, with the member
> open_flags, to be used by callers of the _opts variants of
> bpf_*_get_fd_by_id() to specify the permissions needed for the file
> descriptor to be obtained.
>
> Also, introduce bpf_map_get_fd_by_id_opts(), to let the caller pass a
> bpf_get_fd_opts structure.
>
> Finally, 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 (current behavior).
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/lib/bpf/bpf.c      | 12 +++++++++++-
>  tools/lib/bpf/bpf.h      | 10 ++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 1d49a0352836..4b03063edf1d 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -948,19 +948,29 @@ int bpf_prog_get_fd_by_id(__u32 id)
>         return libbpf_err_errno(fd);
>  }
>
> -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)
>  {
>         const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
>         union bpf_attr attr;
>         int fd;
>
> +       if (!OPTS_VALID(opts, bpf_get_fd_opts))
> +               return libbpf_err(-EINVAL);
> +
>         memset(&attr, 0, attr_sz);
>         attr.map_id = id;
> +       attr.open_flags = OPTS_GET(opts, open_flags, 0);
>
>         fd = sys_bpf_fd(BPF_MAP_GET_FD_BY_ID, &attr, attr_sz);
>         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)
>  {
>         const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 9c50beabdd14..c60eda3c88ad 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -365,7 +365,17 @@ 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);
> +
> +struct bpf_get_fd_opts {

should we call it "bpf_get_fd_by_id_opts" ? kernel command is
specifically about getting object FD by its external ID, so let's
reflect this in the name?

Otherwise it looks good.


> +       size_t sz; /* size of this struct for forward/backward compatibility */
> +       __u32 open_flags; /* permissions requested for the operation on fd */
> +       size_t :0;
> +};
> +#define bpf_get_fd_opts__last_field open_flags
> +
>  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 04fab9f1fdd7..2e665b21d84f 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -371,6 +371,7 @@ LIBBPF_1.0.0 {
>
>  LIBBPF_1.1.0 {
>         global:
> +               bpf_map_get_fd_by_id_opts;
>                 user_ring_buffer__discard;
>                 user_ring_buffer__free;
>                 user_ring_buffer__new;
> --
> 2.25.1
>

[RESEND][PATCH 3/6] libbpf: Introduce bpf_prog_get_fd_by_id_opts()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce bpf_prog_get_fd_by_id_opts(), for symmetry with
bpf_map_get_fd_by_id_opts(), to let the caller pass the newly introduced
data structure bpf_get_fd_opts. Keep the existing bpf_prog_get_fd_by_id(),
and call bpf_prog_get_fd_by_id_opts() with NULL as opts argument, to
prevent setting open_flags.

Currently, the kernel does not support non-zero open_flags for
bpf_prog_get_fd_by_id_opts(), and a call with them will result in an error
returned by the bpf() system call. The caller should always pass zero
open_flags.

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

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 4b03063edf1d..92464c2d1da7 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -935,19 +935,28 @@ 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)
+int bpf_prog_get_fd_by_id_opts(__u32 id, const struct bpf_get_fd_opts *opts)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, attr_sz);
 	attr.prog_id = id;
+	attr.open_flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_PROG_GET_FD_BY_ID, &attr, attr_sz);
 	return libbpf_err_errno(fd);
 }
 
+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_opts(__u32 id,
 			      const struct bpf_get_fd_opts *opts)
 {
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index c60eda3c88ad..595d342fb7f9 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -373,6 +373,8 @@ struct bpf_get_fd_opts {
 };
 #define bpf_get_fd_opts__last_field open_flags
 
+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);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2e665b21d84f..c3604eaa220d 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -372,6 +372,7 @@ LIBBPF_1.0.0 {
 LIBBPF_1.1.0 {
 	global:
 		bpf_map_get_fd_by_id_opts;
+		bpf_prog_get_fd_by_id_opts;
 		user_ring_buffer__discard;
 		user_ring_buffer__free;
 		user_ring_buffer__new;
-- 
2.25.1

[RESEND][PATCH 4/6] libbpf: Introduce bpf_btf_get_fd_by_id_opts()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce bpf_btf_get_fd_by_id_opts(), for symmetry with
bpf_map_get_fd_by_id_opts(), to let the caller pass the newly introduced
data structure bpf_get_fd_opts. Keep the existing bpf_btf_get_fd_by_id(),
and call bpf_btf_get_fd_by_id_opts() with NULL as opts argument, to prevent
setting open_flags.

Currently, the kernel does not support non-zero open_flags for
bpf_btf_get_fd_by_id_opts(), and a call with them will result in an error
returned by the bpf() system call. The caller should always pass zero
open_flags.

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 92464c2d1da7..f43e8c8afbd3 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -980,19 +980,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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, attr_sz);
 	attr.btf_id = id;
+	attr.open_flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_BTF_GET_FD_BY_ID, &attr, attr_sz);
 	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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 595d342fb7f9..559af0b84299 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -379,6 +379,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 c3604eaa220d..7011d5eec67b 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -371,6 +371,7 @@ LIBBPF_1.0.0 {
 
 LIBBPF_1.1.0 {
 	global:
+		bpf_btf_get_fd_by_id_opts;
 		bpf_map_get_fd_by_id_opts;
 		bpf_prog_get_fd_by_id_opts;
 		user_ring_buffer__discard;
-- 
2.25.1

Re: [RESEND][PATCH 4/6] libbpf: Introduce bpf_btf_get_fd_by_id_opts()

[Andrii Nakryiko]

On Tue, Oct 4, 2022 at 6:18 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce bpf_btf_get_fd_by_id_opts(), for symmetry with
> bpf_map_get_fd_by_id_opts(), to let the caller pass the newly introduced
> data structure bpf_get_fd_opts. Keep the existing bpf_btf_get_fd_by_id(),
> and call bpf_btf_get_fd_by_id_opts() with NULL as opts argument, to prevent
> setting open_flags.
>
> Currently, the kernel does not support non-zero open_flags for
> bpf_btf_get_fd_by_id_opts(), and a call with them will result in an error
> returned by the bpf() system call. The caller should always pass zero
> open_flags.
>
> 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 92464c2d1da7..f43e8c8afbd3 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -980,19 +980,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)
>  {
>         const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
>         union bpf_attr attr;
>         int fd;
>
> +       if (!OPTS_VALID(opts, bpf_get_fd_opts))
> +               return libbpf_err(-EINVAL);
> +
>         memset(&attr, 0, attr_sz);
>         attr.btf_id = id;
> +       attr.open_flags = OPTS_GET(opts, open_flags, 0);
>
>         fd = sys_bpf_fd(BPF_BTF_GET_FD_BY_ID, &attr, attr_sz);
>         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)
>  {
>         const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 595d342fb7f9..559af0b84299 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -379,6 +379,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);

It's subjective, but opts variants of APIs, being more "advanced"
usually are listed after their simpler non-opts variants. Can you
please add new _opts APIs after their non-opts variants?

>  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 c3604eaa220d..7011d5eec67b 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -371,6 +371,7 @@ LIBBPF_1.0.0 {
>
>  LIBBPF_1.1.0 {
>         global:
> +               bpf_btf_get_fd_by_id_opts;
>                 bpf_map_get_fd_by_id_opts;
>                 bpf_prog_get_fd_by_id_opts;
>                 user_ring_buffer__discard;
> --
> 2.25.1
>

[RESEND][PATCH 5/6] libbpf: Introduce bpf_link_get_fd_by_id_opts()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce bpf_link_get_fd_by_id_opts(), for symmetry with
bpf_map_get_fd_by_id_opts(), to let the caller pass the newly introduced
data structure bpf_get_fd_opts. Keep the existing bpf_link_get_fd_by_id(),
and call bpf_link_get_fd_by_id_opts() with NULL as opts argument, to
prevent setting open_flags.

Currently, the kernel does not support non-zero open_flags for
bpf_link_get_fd_by_id_opts(), and a call with them will result in an error
returned by the bpf() system call. The caller should always pass zero
open_flags.

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 f43e8c8afbd3..f3d89a2bdb50 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -1003,19 +1003,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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, open_flags);
 	union bpf_attr attr;
 	int fd;
 
+	if (!OPTS_VALID(opts, bpf_get_fd_opts))
+		return libbpf_err(-EINVAL);
+
 	memset(&attr, 0, attr_sz);
 	attr.link_id = id;
+	attr.open_flags = OPTS_GET(opts, open_flags, 0);
 
 	fd = sys_bpf_fd(BPF_LINK_GET_FD_BY_ID, &attr, attr_sz);
 	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)
 {
 	const size_t attr_sz = offsetofend(union bpf_attr, info);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 559af0b84299..834809d74c82 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -382,6 +382,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 7011d5eec67b..71bf5691a689 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -372,6 +372,7 @@ LIBBPF_1.0.0 {
 LIBBPF_1.1.0 {
 	global:
 		bpf_btf_get_fd_by_id_opts;
+		bpf_link_get_fd_by_id_opts;
 		bpf_map_get_fd_by_id_opts;
 		bpf_prog_get_fd_by_id_opts;
 		user_ring_buffer__discard;
-- 
2.25.1

[RESEND][PATCH 6/6] selftests/bpf: Add tests for _opts variants of bpf_*_get_fd_by_id()

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce the data_input map, write-protected with a small eBPF program
implementing the lsm/bpf_map hook.

Then, ensure that bpf_map_get_fd_by_id() and bpf_map_get_fd_by_id_opts()
with NULL opts don't succeed due to requesting read-write access to the
write-protected map. Also, ensure that bpf_map_get_fd_by_id_opts() with
open_flags in opts set to BPF_F_RDONLY instead succeeds.

After obtaining a read-only fd, ensure that only map lookup succeeds and
not update. Ensure that update works only with the read-write fd obtained
at program loading time, when the write protection was not yet enabled.

Finally, ensure that the other _opts variants of bpf_*_get_fd_by_id() don't
work if the BPF_F_RDONLY flag is set in opts (due to the kernel not
handling the open_flags member of bpf_attr).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
 .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
 .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c

diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
index 17e074eb42b8..780a9b2090ad 100644
--- a/tools/testing/selftests/bpf/DENYLIST.s390x
+++ b/tools/testing/selftests/bpf/DENYLIST.s390x
@@ -75,3 +75,4 @@ user_ringbuf                             # failed to find kernel BTF type ID of
 lookup_key                               # JIT does not support calling kernel function                                (kfunc)
 verify_pkcs7_sig                         # JIT does not support calling kernel function                                (kfunc)
 kfunc_dynptr_param                       # JIT does not support calling kernel function                                (kfunc)
+libbpf_get_fd_opts                       # failed to attach: ERROR: strerror_r(-524)=22                                (trampoline)
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
new file mode 100644
index 000000000000..2d4ba8f80d55
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ */
+
+#include <test_progs.h>
+
+#include "test_libbpf_get_fd_opts.skel.h"
+
+void test_libbpf_get_fd_opts(void)
+{
+	struct test_libbpf_get_fd_opts *skel;
+	struct bpf_map_info info_m = { 0 };
+	__u32 len = sizeof(info_m), value;
+	struct bpf_map *map;
+	int ret, zero = 0, fd = -1;
+
+	DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, fd_opts_rdonly,
+		.open_flags = BPF_F_RDONLY,
+	);
+
+	skel = test_libbpf_get_fd_opts__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_libbpf_get_fd_opts__open_and_load"))
+		return;
+
+	ret = test_libbpf_get_fd_opts__attach(skel);
+	if (!ASSERT_OK(ret, "test_libbpf_get_fd_opts__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, &fd_opts_rdonly);
+	if (!ASSERT_GE(fd, 0, "bpf_map_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* Map lookup should work with read-only fd. */
+	ret = bpf_map_lookup_elem(fd, &zero, &value);
+	if (!ASSERT_OK(ret, "bpf_map_lookup_elem"))
+		goto close_prog;
+
+	if (!ASSERT_EQ(value, 0, "map value mismatch"))
+		goto close_prog;
+
+	/* Map update should not work with read-only fd. */
+	ret = bpf_map_update_elem(fd, &zero, &len, BPF_ANY);
+	if (!ASSERT_LT(ret, 0, "bpf_map_update_elem"))
+		goto close_prog;
+
+	/* Map update should work with read-write fd. */
+	ret = bpf_map_update_elem(bpf_map__fd(map), &zero, &len, BPF_ANY);
+	if (!ASSERT_OK(ret, "bpf_map_update_elem"))
+		goto close_prog;
+
+	/* Prog get fd with opts set should not work (no kernel support). */
+	ret = bpf_prog_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	if (!ASSERT_EQ(ret, -EINVAL, "bpf_prog_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* Link get fd with opts set should not work (no kernel support). */
+	ret = bpf_link_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	if (!ASSERT_EQ(ret, -EINVAL, "bpf_link_get_fd_by_id_opts"))
+		goto close_prog;
+
+	/* BTF get fd with opts set should not work (no kernel support). */
+	ret = bpf_btf_get_fd_by_id_opts(0, &fd_opts_rdonly);
+	ASSERT_EQ(ret, -EINVAL, "bpf_btf_get_fd_by_id_opts");
+
+close_prog:
+	close(fd);
+	test_libbpf_get_fd_opts__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
new file mode 100644
index 000000000000..f5ac5f3e8919
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * 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
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u32);
+} data_input SEC(".maps");
+
+char _license[] SEC("license") = "GPL";
+
+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;
+}
-- 
2.25.1

Re: [RESEND][PATCH 6/6] selftests/bpf: Add tests for _opts variants of bpf_*_get_fd_by_id()

[Andrii Nakryiko]

On Tue, Oct 4, 2022 at 6:19 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce the data_input map, write-protected with a small eBPF program
> implementing the lsm/bpf_map hook.
>
> Then, ensure that bpf_map_get_fd_by_id() and bpf_map_get_fd_by_id_opts()
> with NULL opts don't succeed due to requesting read-write access to the
> write-protected map. Also, ensure that bpf_map_get_fd_by_id_opts() with
> open_flags in opts set to BPF_F_RDONLY instead succeeds.
>
> After obtaining a read-only fd, ensure that only map lookup succeeds and
> not update. Ensure that update works only with the read-write fd obtained
> at program loading time, when the write protection was not yet enabled.
>
> Finally, ensure that the other _opts variants of bpf_*_get_fd_by_id() don't
> work if the BPF_F_RDONLY flag is set in opts (due to the kernel not
> handling the open_flags member of bpf_attr).
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
>  .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
>  .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
>
> diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
> index 17e074eb42b8..780a9b2090ad 100644
> --- a/tools/testing/selftests/bpf/DENYLIST.s390x
> +++ b/tools/testing/selftests/bpf/DENYLIST.s390x
> @@ -75,3 +75,4 @@ user_ringbuf                             # failed to find kernel BTF type ID of
>  lookup_key                               # JIT does not support calling kernel function                                (kfunc)
>  verify_pkcs7_sig                         # JIT does not support calling kernel function                                (kfunc)
>  kfunc_dynptr_param                       # JIT does not support calling kernel function                                (kfunc)
> +libbpf_get_fd_opts                       # failed to attach: ERROR: strerror_r(-524)=22                                (trampoline)
> diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
> new file mode 100644
> index 000000000000..2d4ba8f80d55
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> + */
> +
> +#include <test_progs.h>
> +
> +#include "test_libbpf_get_fd_opts.skel.h"
> +
> +void test_libbpf_get_fd_opts(void)
> +{
> +       struct test_libbpf_get_fd_opts *skel;
> +       struct bpf_map_info info_m = { 0 };

1) this 0 is misleading, it's initializing only first field
(explicitly), all the other ones are implicitly initialized with
zeroes as well. In short, just use = {} to zero-initialize structs, in
general.

> +       __u32 len = sizeof(info_m), value;
> +       struct bpf_map *map;
> +       int ret, zero = 0, fd = -1;
> +
> +       DECLARE_LIBBPF_OPTS(bpf_get_fd_opts, fd_opts_rdonly,
> +               .open_flags = BPF_F_RDONLY,
> +       );

LIBBPF_OPTS() macro defines variable, so don't add empty line before
it, it should be part of variable declaration section. Also use
shorter LIBBPF_OPTS() macro instead.


> +
> +       skel = test_libbpf_get_fd_opts__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "test_libbpf_get_fd_opts__open_and_load"))
> +               return;
> +
> +       ret = test_libbpf_get_fd_opts__attach(skel);
> +       if (!ASSERT_OK(ret, "test_libbpf_get_fd_opts__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;

why find_map_by_name, you have that same map as skel->maps.data_input

> +
> +       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;

[...]

> +       /* BTF get fd with opts set should not work (no kernel support). */
> +       ret = bpf_btf_get_fd_by_id_opts(0, &fd_opts_rdonly);
> +       ASSERT_EQ(ret, -EINVAL, "bpf_btf_get_fd_by_id_opts");
> +
> +close_prog:
> +       close(fd);

if (fd >= 0) is missing

> +       test_libbpf_get_fd_opts__destroy(skel);
> +}

[...]

Re: [RESEND][PATCH 0/6] Add _opts variant for bpf_*_get_fd_by_id()

[Andrii Nakryiko]

On Tue, Oct 4, 2022 at 6:18 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> NOTE: resending with libbpf_get_fd_opts test added to deny list for s390x.

it's not a resend if you change something in patches, it's a new
version, please mark it appropriately with v2 instead of [RESEND]

>
> Add the _opts variant for bpf_*_get_fd_by_id() functions, to be able to
> pass to the kernel more options, when requesting a fd of an eBPF object to
> the kernel.
>
> Pass the options through a newly introduced structure, bpf_get_fd_opts,
> which currently contains open_flags (the other two members are for
> compatibility and for padding).
>
> open_flags allows the caller to request specific permissions to access a
> map (e.g. read-only). This is useful for example in the situation where a
> map is write-protected.
>
> Besides patches 2-6, which introduce the new variants and the data
> structure, patch 1 fixes the LIBBPF_1.0.0 declaration in libbpf.map.
>
> Roberto Sassu (6):
>   libbpf: Fix LIBBPF_1.0.0 declaration in libbpf.map
>   libbpf: Define bpf_get_fd_opts and introduce
>     bpf_map_get_fd_by_id_opts()
>   libbpf: Introduce bpf_prog_get_fd_by_id_opts()
>   libbpf: Introduce bpf_btf_get_fd_by_id_opts()
>   libbpf: Introduce bpf_link_get_fd_by_id_opts()
>   selftests/bpf: Add tests for _opts variants of bpf_*_get_fd_by_id()
>
>  tools/lib/bpf/bpf.c                           | 47 +++++++++-
>  tools/lib/bpf/bpf.h                           | 16 ++++
>  tools/lib/bpf/libbpf.map                      |  6 +-
>  tools/testing/selftests/bpf/DENYLIST.s390x    |  1 +
>  .../bpf/prog_tests/libbpf_get_fd_opts.c       | 88 +++++++++++++++++++
>  .../bpf/progs/test_libbpf_get_fd_opts.c       | 36 ++++++++
>  6 files changed, 189 insertions(+), 5 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_get_fd_opts.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_libbpf_get_fd_opts.c
>
> --
> 2.25.1
>