Re: [PATCH v2 1/4] seccomp: Add sysctl to display available actions

On Thu, Feb 2, 2017 at 9:37 PM, Tyler Hicks <tyhicks(a)canonical.com&gt; wrote: > This patch creates a read-only sysctl containing an ordered list of > seccomp actions that the kernel supports. The ordering, from left to > right, is the lowest action value (kill) to the highest action value > (allow). Currently, a read of the sysctl file would return "kill trap > errno trace allow". The contents of this sysctl file can be useful for > userspace code as well as the system administrator. > > The path to the sysctl is: > > /proc/sys/kernel/seccomp/actions_avail > > libseccomp and other userspace code can easily determine which actions > the current kernel supports. The set of actions supported by the current > kernel may be different than the set of action macros found in kernel > headers that were installed where the userspace code was built. This is certainly good: having a discoverable way to detect filter capabilities. I do wonder if it'd still be easier to just expose the max_log sysctl as a numeric value, since the SECCOMP_RET_* values are all part of uapi, so we can't escape their values...

> > In addition, this sysctl will allow system administrators to know which > actions are supported by the kernel and make it easier to configure > exactly what seccomp logs through the audit subsystem. Support for this > level of logging configuration will come in a future patch. > > Signed-off-by: Tyler Hicks <tyhicks(a)canonical.com&gt; > --- > kernel/seccomp.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > index f7ce79a..919ad9f 100644 > --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -16,10 +16,12 @@ > #include <linux/atomic.h> > #include <linux/audit.h> > #include <linux/compat.h> > +#include <linux/kmemleak.h> > #include <linux/sched.h> > #include <linux/seccomp.h> > #include <linux/slab.h> > #include <linux/syscalls.h> > +#include <linux/sysctl.h> > > #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER > #include <asm/syscall.h> > @@ -905,3 +907,51 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off, > return ret; > } > #endif > + > +#ifdef CONFIG_SYSCTL > + > +#define SECCOMP_RET_KILL_NAME "kill" > +#define SECCOMP_RET_TRAP_NAME "trap" > +#define SECCOMP_RET_ERRNO_NAME "errno" > +#define SECCOMP_RET_TRACE_NAME "trace" > +#define SECCOMP_RET_ALLOW_NAME "allow" > + > +static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME " " > + SECCOMP_RET_TRAP_NAME " " > + SECCOMP_RET_ERRNO_NAME " " > + SECCOMP_RET_TRACE_NAME " " > + SECCOMP_RET_ALLOW_NAME; > + > +static struct ctl_path seccomp_sysctl_path[] = { > + { .procname = "kernel", }, > + { .procname = "seccomp", }, > + { } > +}; > + > +static struct ctl_table seccomp_sysctl_table[] = { > + { > + .procname = "actions_avail", > + .data = &seccomp_actions_avail, > + .maxlen = sizeof(seccomp_actions_avail), > + .mode = 0444, > + .proc_handler = proc_dostring, > + }, > + { } > +}; > + > +static int __init seccomp_sysctl_init(void) > +{ > + struct ctl_table_header *hdr; > + > + hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table); > + kmemleak_not_leak(hdr); Will kmemleak complain about this if hdr is saved to a global (or not saved at all)? Also, something should be reported in the failure case...

> + return 0; > +} > + > +#else /* CONFIG_SYSCTL */ > + > +static __init int seccomp_sysctl_init(void) { return 0; } > + > +#endif /* CONFIG_SYSCTL */ > + > +device_initcall(seccomp_sysctl_init) > -- > 2.7.4 > -Kees