* Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
> On 1/21/20 1:23 PM, Dr. David Alan Gilbert (git) wrote:
> > From: Stefan Hajnoczi <stefa...@redhat.com>
> >
> > Only allow system calls that are needed by virtiofsd. All other system
> > calls cause SIGSYS to be directed at the thread and the process will
> > coredump.
> >
> > Restricting system calls reduces the kernel attack surface and limits
> > what the process can do when compromised.
> >
> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > with additional entries by:
> > Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalin...@intel.com>
> > Signed-off-by: Masayoshi Mizuma <m.miz...@jp.fujitsu.com>
> > Signed-off-by: Misono Tomohiro <misono.tomoh...@jp.fujitsu.com>
> > Signed-off-by: piaojun <piao...@huawei.com>
> > Signed-off-by: Vivek Goyal <vgo...@redhat.com>
> > Signed-off-by: Eric Ren <renz...@linux.alibaba.com>
> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > ---
> > Makefile | 2 +-
> > tools/virtiofsd/Makefile.objs | 5 +-
> > tools/virtiofsd/passthrough_ll.c | 2 +
> > tools/virtiofsd/seccomp.c | 150 +++++++++++++++++++++++++++++++
> > tools/virtiofsd/seccomp.h | 14 +++
> > 5 files changed, 171 insertions(+), 2 deletions(-)
> > create mode 100644 tools/virtiofsd/seccomp.c
> > create mode 100644 tools/virtiofsd/seccomp.h
> >
> > diff --git a/Makefile b/Makefile
> > index a87e06ad93..967d59c98a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -327,7 +327,7 @@ HELPERS-y += vhost-user-gpu$(EXESUF)
> > vhost-user-json-y += contrib/vhost-user-gpu/50-qemu-gpu.json
> > endif
> > -ifdef CONFIG_LINUX
> > +ifeq ($(CONFIG_LINUX)$(CONFIG_SECCOMP),yy)
> > HELPERS-y += virtiofsd$(EXESUF)
>
> Something is weird here, because I got:
>
> $ make virtiofsd
> ...
> CC tools/virtiofsd/seccomp.o
> tools/virtiofsd/seccomp.c:14:21: fatal error: seccomp.h: No such file or
> directory
> #include <seccomp.h>
> ^
>
> Indeed I don't have libseccomp installed, ./configure reported:
>
> ...
> QGA MSI support no
> seccomp support no
> coroutine backend ucontext
> coroutine pool yes
> debug stack usage no
> ...
>
> Note also:
>
> $ make print-CONFIG_LINUX
> CONFIG_LINUX=y
> $ make print-CONFIG_SECCOMP
> CONFIG_SECCOMP=
> $ make print-CONFIG_LIBCAP_NG
> CONFIG_LIBCAP_NG=y
> $ make print-HELPERS-y
> HELPERS-y=qemu-bridge-helper
The same thing happens if you uninstall mesa-libgbm-devel and do a
'make vhost-user-gpu'
These ifeq's don't remove the definition of the target, they just remove
it from the HELPERS-y list, so stop it being built on an unqualified
'make' but don't change the behaviour when you explicitly ask for the
target.
Can you try:
diff --git a/Makefile b/Makefile
index ba7e2e5ebc..346a981f0e 100644
--- a/Makefile
+++ b/Makefile
@@ -676,8 +676,9 @@ vhost-user-blk$(EXESUF): $(vhost-user-blk-obj-y)
libvhost-user.a
rdmacm-mux$(EXESUF): LIBS += "-libumad"
rdmacm-mux$(EXESUF): $(rdmacm-mux-obj-y) $(COMMON_LDADDS)
$(call LINK, $^)
-
-ifdef CONFIG_LINUX # relies on Linux-specific syscalls
+#
+# relies on Linux specific syscalls
+ifeq ($(CONFIG_LINUX)$(CONFIG_SECCOMP)$(CONFIG_LIBCAP_NG),yyy)
virtiofsd$(EXESUF): $(virtiofsd-obj-y) libvhost-user.a $(COMMON_LDADDS)
$(call LINK, $^)
endif
> > vhost-user-json-y += tools/virtiofsd/50-qemu-virtiofsd.json
> > endif
> > diff --git a/tools/virtiofsd/Makefile.objs b/tools/virtiofsd/Makefile.objs
> > index 45a807500d..076f667e46 100644
> > --- a/tools/virtiofsd/Makefile.objs
> > +++ b/tools/virtiofsd/Makefile.objs
> > @@ -5,5 +5,8 @@ virtiofsd-obj-y = buffer.o \
> > fuse_signals.o \
> > fuse_virtio.o \
> > helper.o \
> > - passthrough_ll.o
> > + passthrough_ll.o \
> > + seccomp.o
> > +seccomp.o-cflags := $(SECCOMP_CFLAGS)
> > +seccomp.o-libs := $(SECCOMP_LIBS)
> > diff --git a/tools/virtiofsd/passthrough_ll.c
> > b/tools/virtiofsd/passthrough_ll.c
> > index 752beb459a..8748e64f33 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -58,6 +58,7 @@
> > #include <unistd.h>
> > #include "passthrough_helpers.h"
> > +#include "seccomp.h"
> > struct lo_map_elem {
> > union {
> > @@ -2090,6 +2091,7 @@ static void setup_sandbox(struct lo_data *lo, struct
> > fuse_session *se)
> > {
> > setup_namespaces(lo, se);
> > setup_mounts(lo->source);
> > + setup_seccomp();
> > }
> > int main(int argc, char *argv[])
> > diff --git a/tools/virtiofsd/seccomp.c b/tools/virtiofsd/seccomp.c
> > new file mode 100644
> > index 0000000000..2aa4d3cc66
> > --- /dev/null
> > +++ b/tools/virtiofsd/seccomp.c
> > @@ -0,0 +1,150 @@
> > +/*
> > + * Seccomp sandboxing for virtiofsd
> > + *
> > + * Copyright (C) 2019 Red Hat, Inc.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "seccomp.h"
> > +#include "fuse_i.h"
> > +#include "fuse_log.h"
> > +#include <errno.h>
> > +#include <glib.h>
> > +#include <seccomp.h>
> > +#include <stdlib.h>
> > +
> > +/* Bodge for libseccomp 2.4.2 which broke ppoll */
> > +#if !defined(__SNR_ppoll) && defined(__SNR_brk)
> > +#ifdef __NR_ppoll
> > +#define __SNR_ppoll __NR_ppoll
> > +#else
> > +#define __SNR_ppoll __PNR_ppoll
> > +#endif
> > +#endif
> > +
> > +static const int syscall_whitelist[] = {
> > + /* TODO ireg sem*() syscalls */
> > + SCMP_SYS(brk),
> > + SCMP_SYS(capget), /* For CAP_FSETID */
> > + SCMP_SYS(capset),
> > + SCMP_SYS(clock_gettime),
> > + SCMP_SYS(clone),
> > +#ifdef __NR_clone3
> > + SCMP_SYS(clone3),
> > +#endif
> > + SCMP_SYS(close),
> > + SCMP_SYS(copy_file_range),
> > + SCMP_SYS(dup),
> > + SCMP_SYS(eventfd2),
> > + SCMP_SYS(exit),
> > + SCMP_SYS(exit_group),
> > + SCMP_SYS(fallocate),
> > + SCMP_SYS(fchmodat),
> > + SCMP_SYS(fchownat),
> > + SCMP_SYS(fcntl),
> > + SCMP_SYS(fdatasync),
> > + SCMP_SYS(fgetxattr),
> > + SCMP_SYS(flistxattr),
> > + SCMP_SYS(flock),
> > + SCMP_SYS(fremovexattr),
> > + SCMP_SYS(fsetxattr),
> > + SCMP_SYS(fstat),
> > + SCMP_SYS(fstatfs),
> > + SCMP_SYS(fsync),
> > + SCMP_SYS(ftruncate),
> > + SCMP_SYS(futex),
> > + SCMP_SYS(getdents),
> > + SCMP_SYS(getdents64),
> > + SCMP_SYS(getegid),
> > + SCMP_SYS(geteuid),
> > + SCMP_SYS(getpid),
> > + SCMP_SYS(gettid),
> > + SCMP_SYS(gettimeofday),
> > + SCMP_SYS(linkat),
> > + SCMP_SYS(lseek),
> > + SCMP_SYS(madvise),
> > + SCMP_SYS(mkdirat),
> > + SCMP_SYS(mknodat),
> > + SCMP_SYS(mmap),
> > + SCMP_SYS(mprotect),
> > + SCMP_SYS(mremap),
> > + SCMP_SYS(munmap),
> > + SCMP_SYS(newfstatat),
> > + SCMP_SYS(open),
> > + SCMP_SYS(openat),
> > + SCMP_SYS(ppoll),
> > + SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */
> > + SCMP_SYS(preadv),
> > + SCMP_SYS(pread64),
> > + SCMP_SYS(pwritev),
> > + SCMP_SYS(pwrite64),
> > + SCMP_SYS(read),
> > + SCMP_SYS(readlinkat),
> > + SCMP_SYS(recvmsg),
> > + SCMP_SYS(renameat),
> > + SCMP_SYS(renameat2),
> > + SCMP_SYS(rt_sigaction),
> > + SCMP_SYS(rt_sigprocmask),
> > + SCMP_SYS(rt_sigreturn),
> > + SCMP_SYS(sendmsg),
> > + SCMP_SYS(setresgid),
> > + SCMP_SYS(setresuid),
> > +#ifdef __NR_setresgid32
> > + SCMP_SYS(setresgid32),
> > +#endif
> > +#ifdef __NR_setresuid32
> > + SCMP_SYS(setresuid32),
> > +#endif
> > + SCMP_SYS(set_robust_list),
> > + SCMP_SYS(symlinkat),
> > + SCMP_SYS(time), /* Rarely needed, except on static builds */
> > + SCMP_SYS(tgkill),
> > + SCMP_SYS(unlinkat),
> > + SCMP_SYS(utimensat),
> > + SCMP_SYS(write),
> > + SCMP_SYS(writev),
> > +};
> > +
> > +void setup_seccomp(void)
> > +{
> > + scmp_filter_ctx ctx;
> > + size_t i;
> > +
> > +#ifdef SCMP_ACT_KILL_PROCESS
> > + ctx = seccomp_init(SCMP_ACT_KILL_PROCESS);
> > + /* Handle a newer libseccomp but an older kernel */
> > + if (!ctx && errno == EOPNOTSUPP) {
> > + ctx = seccomp_init(SCMP_ACT_TRAP);
> > + }
> > +#else
> > + ctx = seccomp_init(SCMP_ACT_TRAP);
> > +#endif
> > + if (!ctx) {
> > + fuse_log(FUSE_LOG_ERR, "seccomp_init() failed\n");
> > + exit(1);
> > + }
> > +
> > + for (i = 0; i < G_N_ELEMENTS(syscall_whitelist); i++) {
> > + if (seccomp_rule_add(ctx, SCMP_ACT_ALLOW,
> > + syscall_whitelist[i], 0) != 0) {
> > + fuse_log(FUSE_LOG_ERR, "seccomp_rule_add syscall %d",
> > + syscall_whitelist[i]);
> > + exit(1);
> > + }
> > + }
> > +
> > + /* libvhost-user calls this for post-copy migration, we don't need it
> > */
> > + if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOSYS),
> > + SCMP_SYS(userfaultfd), 0) != 0) {
> > + fuse_log(FUSE_LOG_ERR, "seccomp_rule_add userfaultfd failed\n");
> > + exit(1);
> > + }
> > +
> > + if (seccomp_load(ctx) < 0) {
> > + fuse_log(FUSE_LOG_ERR, "seccomp_load() failed\n");
> > + exit(1);
> > + }
> > +
> > + seccomp_release(ctx);
> > +}
> > diff --git a/tools/virtiofsd/seccomp.h b/tools/virtiofsd/seccomp.h
> > new file mode 100644
> > index 0000000000..86bce72652
> > --- /dev/null
> > +++ b/tools/virtiofsd/seccomp.h
> > @@ -0,0 +1,14 @@
> > +/*
> > + * Seccomp sandboxing for virtiofsd
> > + *
> > + * Copyright (C) 2019 Red Hat, Inc.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#ifndef VIRTIOFSD_SECCOMP_H
> > +#define VIRTIOFSD_SECCOMP_H
> > +
> > +void setup_seccomp(void);
> > +
> > +#endif /* VIRTIOFSD_SECCOMP_H */
> >
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
