[libvirt] [PATCH] qemu: clear seccomp capability if TSYNC is not supported by host

[Daniel P. Berrangé]

On Thu, Aug 30, 2018 at 02:09:41PM +0200, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> With qemu <= 3.0, when using "-seccomp on", the seccomp policy is only
> applied to the main thread, the vcpu worker thread and other worker
> threads created after seccomp policy is applied; the seccomp policy is
> not applied to e.g. the RCU thread because it is created before the
> seccomp policy is applied.
> 
> Since qemu commit 70dfabeaa79ba4d7a3b699abe1a047c8012db114 "seccomp:
> set the seccomp filter to all threads", qemu will require seccomp
> TSYNC flag, and will fail to start if the flag isn't available.
> 
> Without it, sandboxing is flawed. Disable seccomp capability if the
> host is not capable of using seccomp TSYNC.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
>  configure.ac                 |  2 +-
>  src/qemu/qemu_capabilities.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index da940e34df..c206de1cad 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -330,7 +330,7 @@ AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
>  
>  dnl Availability of various common headers (non-fatal if missing).
>  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
> -  sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
> +  sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h linux/seccomp.h \
>    sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
>    libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
>  dnl Check whether endian provides handy macros.
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index a075677421..dd62246eb4 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -54,6 +54,10 @@
>  #include <sys/wait.h>
>  #include <stdarg.h>
>  #include <sys/utsname.h>
> +#if defined(HAVE_LINUX_SECCOMP_H) && defined(HAVE_SYS_SYSCALL_H)
> +#include <linux/seccomp.h>
> +#include <sys/syscall.h>
> +#endif
>  
>  #define VIR_FROM_THIS VIR_FROM_QEMU
>  
> @@ -4615,6 +4619,15 @@ virQEMUCapsLogProbeFailure(const char *binary)
>  }
>  
>  
> +#if defined(HAVE_LINUX_SECCOMP_H) && defined(HAVE_SYS_SYSCALL_H)
> +static int
> +virSeccomp(unsigned int op, unsigned int flags, void *args)
> +{
> +        errno = 0;
> +        return syscall(__NR_seccomp, op, flags, args);
> +}
> +#endif
> +
>  virQEMUCapsPtr
>  virQEMUCapsNewForBinaryInternal(virArch hostArch,
>                                  const char *binary,
> @@ -4679,6 +4692,20 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
>              goto error;
>      }
>  
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX)) {
> +        bool have_seccomp = false;
> +#if defined(HAVE_LINUX_SECCOMP_H) && defined(HAVE_SYS_SYSCALL_H)
> +        /* check the TSYNC flag - it returns errno == ENOSYS if unavailable */
> +        if (virSeccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC, NULL) < 0 &&
> +            errno == EFAULT) {
> +            have_seccomp = true;
> +        }
> +#endif
> +        if (!have_seccomp) {
> +            virQEMUCapsClear(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX);
> +        }
> +    }
> +
>   cleanup:
>      VIR_FREE(qmperr);
>      return qemuCaps;

This all looks fine, but it will trigger a latent bug in virQEMUCapsIsValid.

It only checks kernelVersion for match for the -accel kvm case. We need
to check kernelVersion unconditionally for all emulators we cache.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|