Re: [Qemu-devel] [PATCH v3 3/3] seccomp: set the seccomp filter to all threads

[Daniel P . Berrangé]

On Wed, Aug 22, 2018 at 04:29:56PM +0200, Marc-André Lureau wrote:
> 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 and SECCOMP_FILTER_FLAG_TSYNC isn't used.
> 
> This can be verified with
> for task in /proc/`pidof qemu`/task/*; do cat $task/status | grep Secc ; done
> Seccomp:      2
> Seccomp:      0
> Seccomp:      0
> Seccomp:      2
> Seccomp:      2
> Seccomp:      2
> 
> Starting with libseccomp 2.2.0 and kernel >= 3.17, we can use
> seccomp_attr_set(ctx, > SCMP_FLTATR_CTL_TSYNC, 1) to update the policy
> on all threads.
> 
> Do it by default if possible, warn if not possible. Add an option to
> set the tsync behaviour explicitly.
> 
> Note: we can't bump libseccomp to 2.2.0 since it's not available in
> Debian oldstable (2.1.0).
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  qemu-seccomp.c  | 65 +++++++++++++++++++++++++++++++++++++++++++++++--
>  qemu-options.hx |  2 ++
>  2 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index f0c833f3ca..aa23eae970 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -119,6 +119,45 @@ qemu_seccomp(unsigned int operation, unsigned int flags, 
> void *args)
>  #endif
>  }
>  
> +static bool qemu_seccomp_syscall_check(void)
> +{
> +    int rc;
> +
> +    /*
> +     * this is an invalid call because the second argument is non-zero, but
> +     * depending on the errno value of ENOSYS or EINVAL we can guess if the
> +     * seccomp() syscal is supported or not
> +     */
> +    rc = qemu_seccomp(SECCOMP_SET_MODE_STRICT, 1, NULL);
> +    if (rc < 0 && errno == EINVAL) {
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
> +static bool qemu_seccomp_get_default_tsync(void)
> +{
> +    bool tsync = true;
> +
> +    /* TSYNC support was added with the syscall */
> +    if (!qemu_seccomp_syscall_check()) {
> +        error_report("The host kernel doesn't support seccomp TSYNC!");
> +        tsync = false;
> +    }
> +
> +#if !(SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2)
> +    error_report("libseccomp is too old to support TSYNC!");
> +    tsync = false;
> +#endif
> +
> +    if (!tsync) {
> +        error_report("Only the main thread will be filtered by seccomp!");

At this point you might as well not bother using seccomp at all. The
thread that is confined merely needs to scribble something into the
stack of the unconfined thread and now it can do whatever it wants.

IMHO we need to find a way to get the policy to apply to those other
threads.

The RCU thread is tricky as it is spawned from a __constructor__
function, which means it'll be active way before we setup seccomp.

I think we need to figure out a way todo synchronization between
the RCU thread and the seccomp setup code. Could we have a global
variable 'int seccomp_initialized' that we check from the RCU
thread loop - when that toggles to non-zero, the RCU thread can
then call into the seccomp_start() method to activate policy in
its thread. We'd need a synchronous feedback mechansim back to
the main thread, as it must block startup until all the threads
have activated the seccomp filter.

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5515dfaba5..dafacb60c6 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3864,6 +3864,8 @@ Disable set*uid|gid system calls
>  Disable *fork and execve
>  @item address@hidden
>  Disable process affinity and schedular priority
> address@hidden address@hidden
> +Apply seccomp filter to all threads (default is auto, and will warn if fail)

IMHO this should never exist, as setting "tsync" to anything other
than "yes", is akin to just running without any sandbox.

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 :|