* [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests
@ 2022-03-16 1:48 Martin KaFai Lau
2022-03-16 1:48 ` [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall Martin KaFai Lau
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Martin KaFai Lau @ 2022-03-16 1:48 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
After upgrading to the newer libcap (>= 2.60),
the libcap commit aca076443591 ("Make cap_t operations thread safe.")
added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
shift that breaks the assumption made in the "struct libcap" definition
in test_verifier.c.
This set is to remove the libcap dependency from the bpf selftests.
Martin KaFai Lau (3):
bpf: selftests: Add helpers to directly use the capget and capset
syscall
bpf: selftests: Remove libcap usage from test_verifier
bpf: selftests: Remove libcap usage from test_progs
tools/testing/selftests/bpf/Makefile | 8 +-
tools/testing/selftests/bpf/cap_helpers.c | 68 ++++++++++++++
tools/testing/selftests/bpf/cap_helpers.h | 10 +++
.../selftests/bpf/prog_tests/bind_perm.c | 45 ++--------
tools/testing/selftests/bpf/test_verifier.c | 89 ++++++-------------
5 files changed, 118 insertions(+), 102 deletions(-)
create mode 100644 tools/testing/selftests/bpf/cap_helpers.c
create mode 100644 tools/testing/selftests/bpf/cap_helpers.h
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
@ 2022-03-16 1:48 ` Martin KaFai Lau
2022-03-16 6:17 ` John Fastabend
2022-03-16 1:48 ` [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier Martin KaFai Lau
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Martin KaFai Lau @ 2022-03-16 1:48 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
After upgrading to the newer libcap (>= 2.60),
the libcap commit aca076443591 ("Make cap_t operations thread safe.")
added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
shift that breaks the assumption made in the "struct libcap" definition
in test_verifier.c.
The bpf selftest usage only needs to enable and disable the effective
caps of the running task. It is easier to directly syscall the
capget and capset instead. It can also remove the libcap
library dependency.
The cap_helpers.{c,h} is added. One __u64 is used for all CAP_*
bits instead of two __u32.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/testing/selftests/bpf/cap_helpers.c | 68 +++++++++++++++++++++++
tools/testing/selftests/bpf/cap_helpers.h | 10 ++++
2 files changed, 78 insertions(+)
create mode 100644 tools/testing/selftests/bpf/cap_helpers.c
create mode 100644 tools/testing/selftests/bpf/cap_helpers.h
diff --git a/tools/testing/selftests/bpf/cap_helpers.c b/tools/testing/selftests/bpf/cap_helpers.c
new file mode 100644
index 000000000000..e83eab902657
--- /dev/null
+++ b/tools/testing/selftests/bpf/cap_helpers.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/capability.h>
+#include "cap_helpers.h"
+
+/* Avoid including <sys/capability.h> from the libcap-devel package,
+ * so directly declare them here and use them from glibc.
+ */
+int capget(cap_user_header_t header, cap_user_data_t data);
+int capset(cap_user_header_t header, const cap_user_data_t data);
+
+int cap_enable_effective(__u64 caps, __u64 *old_caps)
+{
+ struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
+ struct __user_cap_header_struct hdr = {
+ .version = _LINUX_CAPABILITY_VERSION_3,
+ };
+ __u32 cap0 = caps;
+ __u32 cap1 = caps >> 32;
+ int err;
+
+ err = capget(&hdr, data);
+ if (err)
+ return err;
+
+ if (old_caps)
+ *old_caps = (__u64)(data[1].effective) << 32 | data[0].effective;
+
+ if ((data[0].effective & cap0) == cap0 &&
+ (data[1].effective & cap1) == cap1)
+ return 0;
+
+ data[0].effective |= cap0;
+ data[1].effective |= cap1;
+ err = capset(&hdr, data);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+int cap_disable_effective(__u64 caps, __u64 *old_caps)
+{
+ struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
+ struct __user_cap_header_struct hdr = {
+ .version = _LINUX_CAPABILITY_VERSION_3,
+ };
+ __u32 cap0 = caps;
+ __u32 cap1 = caps >> 32;
+ int err;
+
+ err = capget(&hdr, data);
+ if (err)
+ return err;
+
+ if (old_caps)
+ *old_caps = (__u64)(data[1].effective) << 32 | data[0].effective;
+
+ if (!(data[0].effective & cap0) && !(data[1].effective & cap1))
+ return 0;
+
+ data[0].effective &= ~cap0;
+ data[1].effective &= ~cap1;
+ err = capset(&hdr, data);
+ if (err)
+ return err;
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/cap_helpers.h b/tools/testing/selftests/bpf/cap_helpers.h
new file mode 100644
index 000000000000..0bf29ecd338c
--- /dev/null
+++ b/tools/testing/selftests/bpf/cap_helpers.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __CAP_HELPERS_H
+#define __CAP_HELPERS_H
+
+#include <linux/types.h>
+
+int cap_enable_effective(__u64 caps, __u64 *old_caps);
+int cap_disable_effective(__u64 caps, __u64 *old_caps);
+
+#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
2022-03-16 1:48 ` [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall Martin KaFai Lau
@ 2022-03-16 1:48 ` Martin KaFai Lau
2022-03-17 2:18 ` Shung-Hsi Yu
2022-03-16 1:49 ` [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs Martin KaFai Lau
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Martin KaFai Lau @ 2022-03-16 1:48 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
This patch removes the libcap usage from test_verifier.
The cap_*_effective() helpers added in the earlier patch are
used instead.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/test_verifier.c | 89 ++++++---------------
2 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index fe12b4f5fe20..1c6e55740019 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -195,6 +195,7 @@ $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
CGROUP_HELPERS := $(OUTPUT)/cgroup_helpers.o
TESTING_HELPERS := $(OUTPUT)/testing_helpers.o
TRACE_HELPERS := $(OUTPUT)/trace_helpers.o
+CAP_HELPERS := $(OUTPUT)/cap_helpers.o
$(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS)
$(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS)
@@ -211,7 +212,7 @@ $(OUTPUT)/test_lirc_mode2_user: $(TESTING_HELPERS)
$(OUTPUT)/xdping: $(TESTING_HELPERS)
$(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
$(OUTPUT)/test_maps: $(TESTING_HELPERS)
-$(OUTPUT)/test_verifier: $(TESTING_HELPERS)
+$(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
BPFTOOL ?= $(DEFAULT_BPFTOOL)
$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 92e3465fbae8..091848662b7a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -22,8 +22,7 @@
#include <limits.h>
#include <assert.h>
-#include <sys/capability.h>
-
+#include <linux/capability.h>
#include <linux/unistd.h>
#include <linux/filter.h>
#include <linux/bpf_perf_event.h>
@@ -42,6 +41,7 @@
# define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
# endif
#endif
+#include "cap_helpers.h"
#include "bpf_rand.h"
#include "bpf_util.h"
#include "test_btf.h"
@@ -62,6 +62,10 @@
#define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0)
#define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1)
+/* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
+#define ADMIN_CAPS (1ULL << CAP_NET_ADMIN | \
+ 1ULL << CAP_PERFMON | \
+ 1ULL << CAP_BPF)
#define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled"
static bool unpriv_disabled = false;
static int skips;
@@ -973,47 +977,19 @@ struct libcap {
static int set_admin(bool admin)
{
- cap_t caps;
- /* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
- const cap_value_t cap_net_admin = CAP_NET_ADMIN;
- const cap_value_t cap_sys_admin = CAP_SYS_ADMIN;
- struct libcap *cap;
- int ret = -1;
-
- caps = cap_get_proc();
- if (!caps) {
- perror("cap_get_proc");
- return -1;
- }
- cap = (struct libcap *)caps;
- if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_sys_admin, CAP_CLEAR)) {
- perror("cap_set_flag clear admin");
- goto out;
- }
- if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_admin,
- admin ? CAP_SET : CAP_CLEAR)) {
- perror("cap_set_flag set_or_clear net");
- goto out;
- }
- /* libcap is likely old and simply ignores CAP_BPF and CAP_PERFMON,
- * so update effective bits manually
- */
+ int err;
+
if (admin) {
- cap->data[1].effective |= 1 << (38 /* CAP_PERFMON */ - 32);
- cap->data[1].effective |= 1 << (39 /* CAP_BPF */ - 32);
+ err = cap_enable_effective(ADMIN_CAPS, NULL);
+ if (err)
+ perror("cap_enable_effective(ADMIN_CAPS)");
} else {
- cap->data[1].effective &= ~(1 << (38 - 32));
- cap->data[1].effective &= ~(1 << (39 - 32));
- }
- if (cap_set_proc(caps)) {
- perror("cap_set_proc");
- goto out;
+ err = cap_disable_effective(ADMIN_CAPS, NULL);
+ if (err)
+ perror("cap_disable_effective(ADMIN_CAPS)");
}
- ret = 0;
-out:
- if (cap_free(caps))
- perror("cap_free");
- return ret;
+
+ return err;
}
static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
@@ -1291,31 +1267,18 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
static bool is_admin(void)
{
- cap_flag_value_t net_priv = CAP_CLEAR;
- bool perfmon_priv = false;
- bool bpf_priv = false;
- struct libcap *cap;
- cap_t caps;
-
-#ifdef CAP_IS_SUPPORTED
- if (!CAP_IS_SUPPORTED(CAP_SETFCAP)) {
- perror("cap_get_flag");
- return false;
- }
-#endif
- caps = cap_get_proc();
- if (!caps) {
- perror("cap_get_proc");
+ __u64 caps;
+
+ /* The test checks for finer cap as CAP_NET_ADMIN,
+ * CAP_PERFMON, and CAP_BPF instead of CAP_SYS_ADMIN.
+ * Thus, disable CAP_SYS_ADMIN at the beginning.
+ */
+ if (cap_disable_effective(1ULL << CAP_SYS_ADMIN, &caps)) {
+ perror("cap_disable_effective(CAP_SYS_ADMIN)");
return false;
}
- cap = (struct libcap *)caps;
- bpf_priv = cap->data[1].effective & (1 << (39/* CAP_BPF */ - 32));
- perfmon_priv = cap->data[1].effective & (1 << (38/* CAP_PERFMON */ - 32));
- if (cap_get_flag(caps, CAP_NET_ADMIN, CAP_EFFECTIVE, &net_priv))
- perror("cap_get_flag NET");
- if (cap_free(caps))
- perror("cap_free");
- return bpf_priv && perfmon_priv && net_priv == CAP_SET;
+
+ return (caps & ADMIN_CAPS) == ADMIN_CAPS;
}
static void get_unpriv_disabled()
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
2022-03-16 1:48 ` [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall Martin KaFai Lau
2022-03-16 1:48 ` [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier Martin KaFai Lau
@ 2022-03-16 1:49 ` Martin KaFai Lau
2022-03-16 15:14 ` sdf
2022-03-16 5:36 ` [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Andrii Nakryiko
2022-03-16 6:20 ` John Fastabend
4 siblings, 1 reply; 11+ messages in thread
From: Martin KaFai Lau @ 2022-03-16 1:49 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Stanislav Fomichev
This patch removes the libcap usage from test_progs.
bind_perm.c is the only user. cap_*_effective() helpers added in the
earlier patch are directly used instead.
No other selftest binary is using libcap, so '-lcap' is also removed
from the Makefile.
Cc: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/testing/selftests/bpf/Makefile | 5 ++-
.../selftests/bpf/prog_tests/bind_perm.c | 45 ++++---------------
2 files changed, 12 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1c6e55740019..11f5883636c3 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -25,7 +25,7 @@ CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
-I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
LDFLAGS += $(SAN_CFLAGS)
-LDLIBS += -lcap -lelf -lz -lrt -lpthread
+LDLIBS += -lelf -lz -lrt -lpthread
# Silence some warnings when compiled with clang
ifneq ($(LLVM),)
@@ -480,7 +480,8 @@ TRUNNER_TESTS_DIR := prog_tests
TRUNNER_BPF_PROGS_DIR := progs
TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
network_helpers.c testing_helpers.c \
- btf_helpers.c flow_dissector_load.h
+ btf_helpers.c flow_dissector_load.h \
+ cap_helpers.c
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
ima_setup.sh \
$(wildcard progs/btf_dump_test_case_*.c)
diff --git a/tools/testing/selftests/bpf/prog_tests/bind_perm.c b/tools/testing/selftests/bpf/prog_tests/bind_perm.c
index eac71fbb24ce..6562b5fdcf1e 100644
--- a/tools/testing/selftests/bpf/prog_tests/bind_perm.c
+++ b/tools/testing/selftests/bpf/prog_tests/bind_perm.c
@@ -4,9 +4,10 @@
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
-#include <sys/capability.h>
+#include <linux/capability.h>
#include "test_progs.h"
+#include "cap_helpers.h"
#include "bind_perm.skel.h"
static int duration;
@@ -49,41 +50,11 @@ void try_bind(int family, int port, int expected_errno)
close(fd);
}
-bool cap_net_bind_service(cap_flag_value_t flag)
-{
- const cap_value_t cap_net_bind_service = CAP_NET_BIND_SERVICE;
- cap_flag_value_t original_value;
- bool was_effective = false;
- cap_t caps;
-
- caps = cap_get_proc();
- if (CHECK(!caps, "cap_get_proc", "errno %d", errno))
- goto free_caps;
-
- if (CHECK(cap_get_flag(caps, CAP_NET_BIND_SERVICE, CAP_EFFECTIVE,
- &original_value),
- "cap_get_flag", "errno %d", errno))
- goto free_caps;
-
- was_effective = (original_value == CAP_SET);
-
- if (CHECK(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_bind_service,
- flag),
- "cap_set_flag", "errno %d", errno))
- goto free_caps;
-
- if (CHECK(cap_set_proc(caps), "cap_set_proc", "errno %d", errno))
- goto free_caps;
-
-free_caps:
- CHECK(cap_free(caps), "cap_free", "errno %d", errno);
- return was_effective;
-}
-
void test_bind_perm(void)
{
- bool cap_was_effective;
+ const __u64 net_bind_svc_cap = 1ULL << CAP_NET_BIND_SERVICE;
struct bind_perm *skel;
+ __u64 old_caps = 0;
int cgroup_fd;
if (create_netns())
@@ -105,7 +76,8 @@ void test_bind_perm(void)
if (!ASSERT_OK_PTR(skel, "bind_v6_prog"))
goto close_skeleton;
- cap_was_effective = cap_net_bind_service(CAP_CLEAR);
+ ASSERT_OK(cap_disable_effective(net_bind_svc_cap, &old_caps),
+ "cap_disable_effective");
try_bind(AF_INET, 110, EACCES);
try_bind(AF_INET6, 110, EACCES);
@@ -113,8 +85,9 @@ void test_bind_perm(void)
try_bind(AF_INET, 111, 0);
try_bind(AF_INET6, 111, 0);
- if (cap_was_effective)
- cap_net_bind_service(CAP_SET);
+ if (old_caps & net_bind_svc_cap)
+ ASSERT_OK(cap_enable_effective(net_bind_svc_cap, NULL),
+ "cap_enable_effective");
close_skeleton:
bind_perm__destroy(skel);
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
` (2 preceding siblings ...)
2022-03-16 1:49 ` [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs Martin KaFai Lau
@ 2022-03-16 5:36 ` Andrii Nakryiko
2022-03-16 17:17 ` Martin KaFai Lau
2022-03-16 6:20 ` John Fastabend
4 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2022-03-16 5:36 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Tue, Mar 15, 2022 at 6:48 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> After upgrading to the newer libcap (>= 2.60),
> the libcap commit aca076443591 ("Make cap_t operations thread safe.")
> added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
> shift that breaks the assumption made in the "struct libcap" definition
> in test_verifier.c.
>
> This set is to remove the libcap dependency from the bpf selftests.
>
> Martin KaFai Lau (3):
> bpf: selftests: Add helpers to directly use the capget and capset
> syscall
> bpf: selftests: Remove libcap usage from test_verifier
> bpf: selftests: Remove libcap usage from test_progs
>
Love the clean up and dropping the dependency on libcap! But it
currently breaks CI, probably because of missing CAP_BPF definitions
due to old system headers. Let's add #ifndef CAP_BPF/#define CAP_BPF
XXX/#endif guards for newer capabilities to make it work in CI as
well?
[0] https://github.com/kernel-patches/bpf/runs/5563642266?check_suite_focus=true
> tools/testing/selftests/bpf/Makefile | 8 +-
> tools/testing/selftests/bpf/cap_helpers.c | 68 ++++++++++++++
> tools/testing/selftests/bpf/cap_helpers.h | 10 +++
> .../selftests/bpf/prog_tests/bind_perm.c | 45 ++--------
> tools/testing/selftests/bpf/test_verifier.c | 89 ++++++-------------
> 5 files changed, 118 insertions(+), 102 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.c
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.h
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall
2022-03-16 1:48 ` [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall Martin KaFai Lau
@ 2022-03-16 6:17 ` John Fastabend
0 siblings, 0 replies; 11+ messages in thread
From: John Fastabend @ 2022-03-16 6:17 UTC (permalink / raw)
To: Martin KaFai Lau, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Martin KaFai Lau wrote:
> After upgrading to the newer libcap (>= 2.60),
> the libcap commit aca076443591 ("Make cap_t operations thread safe.")
> added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
> shift that breaks the assumption made in the "struct libcap" definition
> in test_verifier.c.
>
> The bpf selftest usage only needs to enable and disable the effective
> caps of the running task. It is easier to directly syscall the
> capget and capset instead. It can also remove the libcap
> library dependency.
>
> The cap_helpers.{c,h} is added. One __u64 is used for all CAP_*
> bits instead of two __u32.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> tools/testing/selftests/bpf/cap_helpers.c | 68 +++++++++++++++++++++++
> tools/testing/selftests/bpf/cap_helpers.h | 10 ++++
> 2 files changed, 78 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.c
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.h
>
> diff --git a/tools/testing/selftests/bpf/cap_helpers.c b/tools/testing/selftests/bpf/cap_helpers.c
> new file mode 100644
> index 000000000000..e83eab902657
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/cap_helpers.c
LGTM
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
` (3 preceding siblings ...)
2022-03-16 5:36 ` [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Andrii Nakryiko
@ 2022-03-16 6:20 ` John Fastabend
4 siblings, 0 replies; 11+ messages in thread
From: John Fastabend @ 2022-03-16 6:20 UTC (permalink / raw)
To: Martin KaFai Lau, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Martin KaFai Lau wrote:
> After upgrading to the newer libcap (>= 2.60),
> the libcap commit aca076443591 ("Make cap_t operations thread safe.")
> added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
> shift that breaks the assumption made in the "struct libcap" definition
> in test_verifier.c.
>
> This set is to remove the libcap dependency from the bpf selftests.
>
> Martin KaFai Lau (3):
> bpf: selftests: Add helpers to directly use the capget and capset
> syscall
> bpf: selftests: Remove libcap usage from test_verifier
> bpf: selftests: Remove libcap usage from test_progs
>
> tools/testing/selftests/bpf/Makefile | 8 +-
> tools/testing/selftests/bpf/cap_helpers.c | 68 ++++++++++++++
> tools/testing/selftests/bpf/cap_helpers.h | 10 +++
> .../selftests/bpf/prog_tests/bind_perm.c | 45 ++--------
> tools/testing/selftests/bpf/test_verifier.c | 89 ++++++-------------
> 5 files changed, 118 insertions(+), 102 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.c
> create mode 100644 tools/testing/selftests/bpf/cap_helpers.h
>
> --
> 2.30.2
>
For the series,
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs
2022-03-16 1:49 ` [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs Martin KaFai Lau
@ 2022-03-16 15:14 ` sdf
0 siblings, 0 replies; 11+ messages in thread
From: sdf @ 2022-03-16 15:14 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
On 03/15, Martin KaFai Lau wrote:
> This patch removes the libcap usage from test_progs.
> bind_perm.c is the only user. cap_*_effective() helpers added in the
> earlier patch are directly used instead.
> No other selftest binary is using libcap, so '-lcap' is also removed
> from the Makefile.
> Cc: Stanislav Fomichev <sdf@google.com>
LGTM!
Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> tools/testing/selftests/bpf/Makefile | 5 ++-
> .../selftests/bpf/prog_tests/bind_perm.c | 45 ++++---------------
> 2 files changed, 12 insertions(+), 38 deletions(-)
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 1c6e55740019..11f5883636c3 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -25,7 +25,7 @@ CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS)
> $(SAN_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> LDFLAGS += $(SAN_CFLAGS)
> -LDLIBS += -lcap -lelf -lz -lrt -lpthread
> +LDLIBS += -lelf -lz -lrt -lpthread
> # Silence some warnings when compiled with clang
> ifneq ($(LLVM),)
> @@ -480,7 +480,8 @@ TRUNNER_TESTS_DIR := prog_tests
> TRUNNER_BPF_PROGS_DIR := progs
> TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
> network_helpers.c testing_helpers.c \
> - btf_helpers.c flow_dissector_load.h
> + btf_helpers.c flow_dissector_load.h \
> + cap_helpers.c
> TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
> ima_setup.sh \
> $(wildcard progs/btf_dump_test_case_*.c)
> diff --git a/tools/testing/selftests/bpf/prog_tests/bind_perm.c
> b/tools/testing/selftests/bpf/prog_tests/bind_perm.c
> index eac71fbb24ce..6562b5fdcf1e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bind_perm.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bind_perm.c
> @@ -4,9 +4,10 @@
> #include <stdlib.h>
> #include <sys/types.h>
> #include <sys/socket.h>
> -#include <sys/capability.h>
> +#include <linux/capability.h>
> #include "test_progs.h"
> +#include "cap_helpers.h"
> #include "bind_perm.skel.h"
> static int duration;
> @@ -49,41 +50,11 @@ void try_bind(int family, int port, int
> expected_errno)
> close(fd);
> }
> -bool cap_net_bind_service(cap_flag_value_t flag)
> -{
> - const cap_value_t cap_net_bind_service = CAP_NET_BIND_SERVICE;
> - cap_flag_value_t original_value;
> - bool was_effective = false;
> - cap_t caps;
> -
> - caps = cap_get_proc();
> - if (CHECK(!caps, "cap_get_proc", "errno %d", errno))
> - goto free_caps;
> -
> - if (CHECK(cap_get_flag(caps, CAP_NET_BIND_SERVICE, CAP_EFFECTIVE,
> - &original_value),
> - "cap_get_flag", "errno %d", errno))
> - goto free_caps;
> -
> - was_effective = (original_value == CAP_SET);
> -
> - if (CHECK(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_bind_service,
> - flag),
> - "cap_set_flag", "errno %d", errno))
> - goto free_caps;
> -
> - if (CHECK(cap_set_proc(caps), "cap_set_proc", "errno %d", errno))
> - goto free_caps;
> -
> -free_caps:
> - CHECK(cap_free(caps), "cap_free", "errno %d", errno);
> - return was_effective;
> -}
> -
> void test_bind_perm(void)
> {
> - bool cap_was_effective;
> + const __u64 net_bind_svc_cap = 1ULL << CAP_NET_BIND_SERVICE;
> struct bind_perm *skel;
> + __u64 old_caps = 0;
> int cgroup_fd;
> if (create_netns())
> @@ -105,7 +76,8 @@ void test_bind_perm(void)
> if (!ASSERT_OK_PTR(skel, "bind_v6_prog"))
> goto close_skeleton;
> - cap_was_effective = cap_net_bind_service(CAP_CLEAR);
> + ASSERT_OK(cap_disable_effective(net_bind_svc_cap, &old_caps),
> + "cap_disable_effective");
> try_bind(AF_INET, 110, EACCES);
> try_bind(AF_INET6, 110, EACCES);
> @@ -113,8 +85,9 @@ void test_bind_perm(void)
> try_bind(AF_INET, 111, 0);
> try_bind(AF_INET6, 111, 0);
> - if (cap_was_effective)
> - cap_net_bind_service(CAP_SET);
> + if (old_caps & net_bind_svc_cap)
> + ASSERT_OK(cap_enable_effective(net_bind_svc_cap, NULL),
> + "cap_enable_effective");
> close_skeleton:
> bind_perm__destroy(skel);
> --
> 2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests
2022-03-16 5:36 ` [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Andrii Nakryiko
@ 2022-03-16 17:17 ` Martin KaFai Lau
0 siblings, 0 replies; 11+ messages in thread
From: Martin KaFai Lau @ 2022-03-16 17:17 UTC (permalink / raw)
To: Andrii Nakryiko, John Fastabend, Stanislav Fomichev
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Tue, Mar 15, 2022 at 10:36:48PM -0700, Andrii Nakryiko wrote:
> On Tue, Mar 15, 2022 at 6:48 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > After upgrading to the newer libcap (>= 2.60),
> > the libcap commit aca076443591 ("Make cap_t operations thread safe.")
> > added a "__u8 mutex;" to the "struct _cap_struct". It caused a few byte
> > shift that breaks the assumption made in the "struct libcap" definition
> > in test_verifier.c.
> >
> > This set is to remove the libcap dependency from the bpf selftests.
> >
> > Martin KaFai Lau (3):
> > bpf: selftests: Add helpers to directly use the capget and capset
> > syscall
> > bpf: selftests: Remove libcap usage from test_verifier
> > bpf: selftests: Remove libcap usage from test_progs
> >
>
> Love the clean up and dropping the dependency on libcap! But it
> currently breaks CI, probably because of missing CAP_BPF definitions
> due to old system headers. Let's add #ifndef CAP_BPF/#define CAP_BPF
> XXX/#endif guards for newer capabilities to make it work in CI as
> well?
will spin v2. Thanks everyone for the review !
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier
2022-03-16 1:48 ` [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier Martin KaFai Lau
@ 2022-03-17 2:18 ` Shung-Hsi Yu
2022-03-17 7:04 ` Shung-Hsi Yu
0 siblings, 1 reply; 11+ messages in thread
From: Shung-Hsi Yu @ 2022-03-17 2:18 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
On Tue, Mar 15, 2022 at 06:48:54PM -0700, Martin KaFai Lau wrote:
> This patch removes the libcap usage from test_verifier.
> The cap_*_effective() helpers added in the earlier patch are
> used instead.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> tools/testing/selftests/bpf/Makefile | 3 +-
> tools/testing/selftests/bpf/test_verifier.c | 89 ++++++---------------
> 2 files changed, 28 insertions(+), 64 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index fe12b4f5fe20..1c6e55740019 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -195,6 +195,7 @@ $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
> CGROUP_HELPERS := $(OUTPUT)/cgroup_helpers.o
> TESTING_HELPERS := $(OUTPUT)/testing_helpers.o
> TRACE_HELPERS := $(OUTPUT)/trace_helpers.o
> +CAP_HELPERS := $(OUTPUT)/cap_helpers.o
>
> $(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS)
> $(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS)
> @@ -211,7 +212,7 @@ $(OUTPUT)/test_lirc_mode2_user: $(TESTING_HELPERS)
> $(OUTPUT)/xdping: $(TESTING_HELPERS)
> $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> -$(OUTPUT)/test_verifier: $(TESTING_HELPERS)
> +$(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
>
> BPFTOOL ?= $(DEFAULT_BPFTOOL)
> $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 92e3465fbae8..091848662b7a 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -22,8 +22,7 @@
> #include <limits.h>
> #include <assert.h>
>
> -#include <sys/capability.h>
> -
> +#include <linux/capability.h>
> #include <linux/unistd.h>
> #include <linux/filter.h>
> #include <linux/bpf_perf_event.h>
> @@ -42,6 +41,7 @@
> # define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
> # endif
> #endif
> +#include "cap_helpers.h"
> #include "bpf_rand.h"
> #include "bpf_util.h"
> #include "test_btf.h"
> @@ -62,6 +62,10 @@
> #define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0)
> #define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1)
>
> +/* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
> +#define ADMIN_CAPS (1ULL << CAP_NET_ADMIN | \
> + 1ULL << CAP_PERFMON | \
> + 1ULL << CAP_BPF)
> #define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled"
> static bool unpriv_disabled = false;
> static int skips;
> @@ -973,47 +977,19 @@ struct libcap {
>
> static int set_admin(bool admin)
> {
> - cap_t caps;
> - /* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
> - const cap_value_t cap_net_admin = CAP_NET_ADMIN;
> - const cap_value_t cap_sys_admin = CAP_SYS_ADMIN;
> - struct libcap *cap;
> - int ret = -1;
> -
> - caps = cap_get_proc();
> - if (!caps) {
> - perror("cap_get_proc");
> - return -1;
> - }
> - cap = (struct libcap *)caps;
> - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_sys_admin, CAP_CLEAR)) {
> - perror("cap_set_flag clear admin");
> - goto out;
> - }
> - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_admin,
> - admin ? CAP_SET : CAP_CLEAR)) {
> - perror("cap_set_flag set_or_clear net");
> - goto out;
> - }
> - /* libcap is likely old and simply ignores CAP_BPF and CAP_PERFMON,
> - * so update effective bits manually
> - */
> + int err;
> +
> if (admin) {
> - cap->data[1].effective |= 1 << (38 /* CAP_PERFMON */ - 32);
> - cap->data[1].effective |= 1 << (39 /* CAP_BPF */ - 32);
> + err = cap_enable_effective(ADMIN_CAPS, NULL);
> + if (err)
> + perror("cap_enable_effective(ADMIN_CAPS)");
> } else {
> - cap->data[1].effective &= ~(1 << (38 - 32));
> - cap->data[1].effective &= ~(1 << (39 - 32));
> - }
> - if (cap_set_proc(caps)) {
> - perror("cap_set_proc");
> - goto out;
> + err = cap_disable_effective(ADMIN_CAPS, NULL);
> + if (err)
> + perror("cap_disable_effective(ADMIN_CAPS)");
> }
> - ret = 0;
> -out:
> - if (cap_free(caps))
> - perror("cap_free");
> - return ret;
> +
> + return err;
> }
>
> static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
> @@ -1291,31 +1267,18 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
>
> static bool is_admin(void)
> {
> - cap_flag_value_t net_priv = CAP_CLEAR;
> - bool perfmon_priv = false;
> - bool bpf_priv = false;
> - struct libcap *cap;
> - cap_t caps;
> -
> -#ifdef CAP_IS_SUPPORTED
> - if (!CAP_IS_SUPPORTED(CAP_SETFCAP)) {
> - perror("cap_get_flag");
> - return false;
> - }
> -#endif
> - caps = cap_get_proc();
> - if (!caps) {
> - perror("cap_get_proc");
> + __u64 caps;
> +
> + /* The test checks for finer cap as CAP_NET_ADMIN,
> + * CAP_PERFMON, and CAP_BPF instead of CAP_SYS_ADMIN.
> + * Thus, disable CAP_SYS_ADMIN at the beginning.
> + */
> + if (cap_disable_effective(1ULL << CAP_SYS_ADMIN, &caps)) {
> + perror("cap_disable_effective(CAP_SYS_ADMIN)");
> return false;
> }
Seems slightly strange for a is_* function to have the side effect of
disabling capability, also, this change of behavior in is_admin() is not in
the commit message.
Maybe a better place to disable CAP_SYS_ADMIN is at the beginning of main()?
That seems to be the only place is_admin() is called.
> - cap = (struct libcap *)caps;
> - bpf_priv = cap->data[1].effective & (1 << (39/* CAP_BPF */ - 32));
> - perfmon_priv = cap->data[1].effective & (1 << (38/* CAP_PERFMON */ - 32));
> - if (cap_get_flag(caps, CAP_NET_ADMIN, CAP_EFFECTIVE, &net_priv))
> - perror("cap_get_flag NET");
> - if (cap_free(caps))
> - perror("cap_free");
> - return bpf_priv && perfmon_priv && net_priv == CAP_SET;
> +
> + return (caps & ADMIN_CAPS) == ADMIN_CAPS;
> }
>
> static void get_unpriv_disabled()
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier
2022-03-17 2:18 ` Shung-Hsi Yu
@ 2022-03-17 7:04 ` Shung-Hsi Yu
0 siblings, 0 replies; 11+ messages in thread
From: Shung-Hsi Yu @ 2022-03-17 7:04 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
On Thu, Mar 17, 2022 at 10:18:45AM +0800, Shung-Hsi Yu wrote:
> On Tue, Mar 15, 2022 at 06:48:54PM -0700, Martin KaFai Lau wrote:
> > This patch removes the libcap usage from test_verifier.
> > The cap_*_effective() helpers added in the earlier patch are
> > used instead.
> >
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 3 +-
> > tools/testing/selftests/bpf/test_verifier.c | 89 ++++++---------------
> > 2 files changed, 28 insertions(+), 64 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index fe12b4f5fe20..1c6e55740019 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -195,6 +195,7 @@ $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
> > CGROUP_HELPERS := $(OUTPUT)/cgroup_helpers.o
> > TESTING_HELPERS := $(OUTPUT)/testing_helpers.o
> > TRACE_HELPERS := $(OUTPUT)/trace_helpers.o
> > +CAP_HELPERS := $(OUTPUT)/cap_helpers.o
> >
> > $(OUTPUT)/test_dev_cgroup: $(CGROUP_HELPERS) $(TESTING_HELPERS)
> > $(OUTPUT)/test_skb_cgroup_id_user: $(CGROUP_HELPERS) $(TESTING_HELPERS)
> > @@ -211,7 +212,7 @@ $(OUTPUT)/test_lirc_mode2_user: $(TESTING_HELPERS)
> > $(OUTPUT)/xdping: $(TESTING_HELPERS)
> > $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > -$(OUTPUT)/test_verifier: $(TESTING_HELPERS)
> > +$(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> >
> > BPFTOOL ?= $(DEFAULT_BPFTOOL)
> > $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> > index 92e3465fbae8..091848662b7a 100644
> > --- a/tools/testing/selftests/bpf/test_verifier.c
> > +++ b/tools/testing/selftests/bpf/test_verifier.c
> > @@ -22,8 +22,7 @@
> > #include <limits.h>
> > #include <assert.h>
> >
> > -#include <sys/capability.h>
> > -
> > +#include <linux/capability.h>
> > #include <linux/unistd.h>
> > #include <linux/filter.h>
> > #include <linux/bpf_perf_event.h>
> > @@ -42,6 +41,7 @@
> > # define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
> > # endif
> > #endif
> > +#include "cap_helpers.h"
> > #include "bpf_rand.h"
> > #include "bpf_util.h"
> > #include "test_btf.h"
> > @@ -62,6 +62,10 @@
> > #define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0)
> > #define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1)
> >
> > +/* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
> > +#define ADMIN_CAPS (1ULL << CAP_NET_ADMIN | \
> > + 1ULL << CAP_PERFMON | \
> > + 1ULL << CAP_BPF)
> > #define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled"
> > static bool unpriv_disabled = false;
> > static int skips;
> > @@ -973,47 +977,19 @@ struct libcap {
> >
> > static int set_admin(bool admin)
> > {
> > - cap_t caps;
> > - /* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */
> > - const cap_value_t cap_net_admin = CAP_NET_ADMIN;
> > - const cap_value_t cap_sys_admin = CAP_SYS_ADMIN;
> > - struct libcap *cap;
> > - int ret = -1;
> > -
> > - caps = cap_get_proc();
> > - if (!caps) {
> > - perror("cap_get_proc");
> > - return -1;
> > - }
> > - cap = (struct libcap *)caps;
> > - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_sys_admin, CAP_CLEAR)) {
> > - perror("cap_set_flag clear admin");
> > - goto out;
> > - }
> > - if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_admin,
> > - admin ? CAP_SET : CAP_CLEAR)) {
> > - perror("cap_set_flag set_or_clear net");
> > - goto out;
> > - }
> > - /* libcap is likely old and simply ignores CAP_BPF and CAP_PERFMON,
> > - * so update effective bits manually
> > - */
> > + int err;
> > +
> > if (admin) {
> > - cap->data[1].effective |= 1 << (38 /* CAP_PERFMON */ - 32);
> > - cap->data[1].effective |= 1 << (39 /* CAP_BPF */ - 32);
> > + err = cap_enable_effective(ADMIN_CAPS, NULL);
> > + if (err)
> > + perror("cap_enable_effective(ADMIN_CAPS)");
> > } else {
> > - cap->data[1].effective &= ~(1 << (38 - 32));
> > - cap->data[1].effective &= ~(1 << (39 - 32));
> > - }
> > - if (cap_set_proc(caps)) {
> > - perror("cap_set_proc");
> > - goto out;
> > + err = cap_disable_effective(ADMIN_CAPS, NULL);
> > + if (err)
> > + perror("cap_disable_effective(ADMIN_CAPS)");
> > }
> > - ret = 0;
> > -out:
> > - if (cap_free(caps))
> > - perror("cap_free");
> > - return ret;
> > +
> > + return err;
> > }
> >
> > static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
> > @@ -1291,31 +1267,18 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> >
> > static bool is_admin(void)
> > {
> > - cap_flag_value_t net_priv = CAP_CLEAR;
> > - bool perfmon_priv = false;
> > - bool bpf_priv = false;
> > - struct libcap *cap;
> > - cap_t caps;
> > -
> > -#ifdef CAP_IS_SUPPORTED
> > - if (!CAP_IS_SUPPORTED(CAP_SETFCAP)) {
> > - perror("cap_get_flag");
> > - return false;
> > - }
> > -#endif
> > - caps = cap_get_proc();
> > - if (!caps) {
> > - perror("cap_get_proc");
> > + __u64 caps;
> > +
> > + /* The test checks for finer cap as CAP_NET_ADMIN,
> > + * CAP_PERFMON, and CAP_BPF instead of CAP_SYS_ADMIN.
> > + * Thus, disable CAP_SYS_ADMIN at the beginning.
> > + */
> > + if (cap_disable_effective(1ULL << CAP_SYS_ADMIN, &caps)) {
> > + perror("cap_disable_effective(CAP_SYS_ADMIN)");
> > return false;
> > }
>
> Seems slightly strange for a is_* function to have the side effect of
> disabling capability, also, this change of behavior in is_admin() is not in
> the commit message.
>
> Maybe a better place to disable CAP_SYS_ADMIN is at the beginning of main()?
> That seems to be the only place is_admin() is called.
Just realized there's a v2 and it's already merged. Sorry for the noise.
Shung-Hsi
> > - cap = (struct libcap *)caps;
> > - bpf_priv = cap->data[1].effective & (1 << (39/* CAP_BPF */ - 32));
> > - perfmon_priv = cap->data[1].effective & (1 << (38/* CAP_PERFMON */ - 32));
> > - if (cap_get_flag(caps, CAP_NET_ADMIN, CAP_EFFECTIVE, &net_priv))
> > - perror("cap_get_flag NET");
> > - if (cap_free(caps))
> > - perror("cap_free");
> > - return bpf_priv && perfmon_priv && net_priv == CAP_SET;
> > +
> > + return (caps & ADMIN_CAPS) == ADMIN_CAPS;
> > }
> >
> > static void get_unpriv_disabled()
> > --
> > 2.30.2
> >
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-17 7:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 1:48 [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Martin KaFai Lau
2022-03-16 1:48 ` [PATCH bpf-next 1/3] bpf: selftests: Add helpers to directly use the capget and capset syscall Martin KaFai Lau
2022-03-16 6:17 ` John Fastabend
2022-03-16 1:48 ` [PATCH bpf-next 2/3] bpf: selftests: Remove libcap usage from test_verifier Martin KaFai Lau
2022-03-17 2:18 ` Shung-Hsi Yu
2022-03-17 7:04 ` Shung-Hsi Yu
2022-03-16 1:49 ` [PATCH bpf-next 3/3] bpf: selftests: Remove libcap usage from test_progs Martin KaFai Lau
2022-03-16 15:14 ` sdf
2022-03-16 5:36 ` [PATCH bpf-next 0/3] Remove libcap dependency from bpf selftests Andrii Nakryiko
2022-03-16 17:17 ` Martin KaFai Lau
2022-03-16 6:20 ` John Fastabend
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.