| Message ID | 20220316014854.2257030-1-kafai@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Remove libcap dependency from bpf selftests | expand |
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 > >
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 > > > >
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()
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(-)