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@xxxxxx>
> > ---
> > 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
> >
> >
