On Thu, Aug 30, 2018 at 02:09:41PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
