From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Stancek Date: Fri, 9 Aug 2019 10:42:18 -0400 (EDT) Subject: [LTP] [RFC PATCH 1/1] capability: Introduce capability API In-Reply-To: <20190809122741.GC32115@rei.lan> References: <20190808153825.18363-1-rpalethorpe@suse.com> <20190808153825.18363-2-rpalethorpe@suse.com> <20190809122741.GC32115@rei.lan> Message-ID: <1048151188.5768032.1565361738465.JavaMail.zimbra@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it ----- Original Message ----- > Hi! > > include/tst_capability.h | 56 +++++++++++++++++++++++++++++ > > include/tst_test.h | 6 ++++ > > lib/tst_capability.c | 78 ++++++++++++++++++++++++++++++++++++++++ > > lib/tst_test.c | 3 ++ > > 4 files changed, 143 insertions(+) > > create mode 100644 include/tst_capability.h > > create mode 100644 lib/tst_capability.c > > This is missing a documentation in the test-writing-guidelines I do > expect that you will add that later on. > > > diff --git a/include/tst_capability.h b/include/tst_capability.h > > new file mode 100644 > > index 000000000..6342b667e > > --- /dev/null > > +++ b/include/tst_capability.h > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (c) 2019 Richard Palethorpe > > + */ > > +/** > > + * @file tst_capability.h > > + * > > + * Limited capability operations without libcap. > > + */ > > + > > +#include > > + > > +#include "lapi/syscalls.h" > ^ > What is this needed for here? > > > +#ifndef TST_CAPABILITY_H > > +#define TST_CAPABILITY_H > > + > > +#ifndef CAP_SYS_ADMIN > > +# define CAP_SYS_ADMIN 21 > > +#endif > > > > +#ifndef CAP_TO_MASK > > +# define CAP_TO_MASK(x) (1 << ((x) & 31)) > > +#endif > > These bits should probably go to lapi/capability.h > > > +#define TST_DROP 1 > > +#define TST_REQUIRE 1 << 1 > > Maybe these should be TST_CAP_DROP and TST_CAP_REQ > > > +#define TST_CAP(action, capability) {action, capability, #capability} > > + > > +struct tst_cap_user_header { > > + uint32_t version; > > + int pid; > > +}; > > + > > +struct tst_cap_user_data { > > + uint32_t effective; > > + uint32_t permitted; > > + uint32_t inheritable; > > +}; > > This two should probably go to lapi as well. > > > +struct tst_cap { > > + uint32_t action; > > + uint32_t id; > > + char *name; > > +}; > > I wonder if we should build a table of capability names for translation > just as we do errnos and signals instead of storing the name here. But I > guess that unless we will need a function to translate capabilitities > into strings on runtime this approach is simpler. > > > +int tst_capget(struct tst_cap_user_header *hdr, > > + struct tst_cap_user_data *data); > > +int tst_capset(struct tst_cap_user_header *hdr, > > + const struct tst_cap_user_data *data); > > + > > +void tst_cap_action(struct tst_cap *cap); > > +void tst_cap_setup(struct tst_cap *cap); > > + > > +#endif > > diff --git a/include/tst_test.h b/include/tst_test.h > > index cdeaf6ad0..84acf2c59 100644 > > --- a/include/tst_test.h > > +++ b/include/tst_test.h > > @@ -36,6 +36,7 @@ > > #include "tst_sys_conf.h" > > #include "tst_coredump.h" > > #include "tst_buffers.h" > > +#include "tst_capability.h" > > > > /* > > * Reports testcase result. > > @@ -206,6 +207,11 @@ struct tst_test { > > * NULL-terminated array to be allocated buffers. > > */ > > struct tst_buffers *bufs; > > + > > + /* > > + * NULL-terminated array of capability settings > > + */ > > + struct tst_cap *caps; > > }; > > > > /* > > diff --git a/lib/tst_capability.c b/lib/tst_capability.c > > new file mode 100644 > > index 000000000..d229491ae > > --- /dev/null > > +++ b/lib/tst_capability.c > > @@ -0,0 +1,78 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (c) 2019 Richard Palethorpe > > + */ > > + > > +#define TST_NO_DEFAULT_MAIN > > +#include "tst_test.h" > > +#include "tst_capability.h" > > + > > +int tst_capget(struct tst_cap_user_header *hdr, > > + struct tst_cap_user_data *data) > > +{ > > + return tst_syscall(__NR_capget, hdr, data); > > +} > > + > > +int tst_capset(struct tst_cap_user_header *hdr, > > + const struct tst_cap_user_data *data) > > +{ > > + return tst_syscall(__NR_capset, hdr, data); > > +} > > + > > +void tst_cap_action(struct tst_cap *cap) > > +{ > > + struct tst_cap_user_header hdr = { > > + .version = 0x20080522, > > + .pid = tst_syscall(__NR_gettid), > > + }; > > + struct tst_cap_user_data cur = { 0 }; > > + struct tst_cap_user_data new = { 0 }; > > + uint32_t mask = CAP_TO_MASK(cap->id); > > + uint32_t act = cap->action; > > + > > + if (tst_capget(&hdr, &cur)) > > + tst_brk(TBROK | TTERRNO, "tst_capget()"); > > + > > + new = cur; > > + > > + switch (act) { > > + case TST_DROP: > > + if (cur.effective & mask) { > > + tst_res(TINFO, "Dropping %s(%d)", > > + cap->name, cap->id); > > + new.effective &= ~mask; > > + new.permitted &= ~mask; > > + new.inheritable &= ~mask; > > + } > > + break; > > + case TST_REQUIRE: > > + if (cur.permitted ^ mask) { > > + tst_brk(TCONF, "Need %s(%d)", > > + cap->name, cap->id); > > + } else if (cur.effective ^ mask) { > > + tst_res(TINFO, "Permitting %s(%d)", > > + cap->name, cap->id); > > + new.effective |= mask; > > + new.inheritable |= mask; > > + } > > + break; > > + default: > > + tst_brk(TBROK, "Unrecognised action %d", cap->action); > > + } > > + > > + if (cur.effective != new.effective) { > > + if (tst_capset(&hdr, &new)) > > + tst_brk(TBROK | TERRNO, "tst_capset(%s)", cap->name); > > + } else { > > + tst_res(TINFO, "No capability changes needed"); > > + } > > +} > > + > > +void tst_cap_setup(struct tst_cap *caps) > > +{ > > + struct tst_cap *cap; > > + > > + for (cap = caps; cap->action; cap++) { > > + tst_cap_action(cap); > > + } > > No need for the curly braces here :-). > > > +} > > diff --git a/lib/tst_test.c b/lib/tst_test.c > > index 8dc71dbb3..62e54d071 100644 > > --- a/lib/tst_test.c > > +++ b/lib/tst_test.c > > @@ -893,6 +893,9 @@ static void do_test_setup(void) > > > > if (main_pid != getpid()) > > tst_brk(TBROK, "Runaway child in setup()!"); > > + > > + if (tst_test->caps) > > + tst_cap_setup(tst_test->caps); > > } > > Other than the minor things and missing docs this patch looks good to me. +1, but please rebase it on latest master.