[Qemu-devel] [PATCH 0/2] Enable seccomp on MIPS

[Qemu-devel] [PATCH 0/2] Enable seccomp on MIPS

[James Hogan]

These patches enable seccomp sandboxing on MIPS.

libseccomp has supported MIPS since 2.2.0, but cacheflush isn't included
in the whitelist until libseccomp 2.2.3 since thats when it was enabled
for ARM. The first patch fixes that so that it will work with MIPS right
back to 2.2.0.

Finally the second patch enables seccomp in the configure script for
MIPS since libseccomp 2.2.0.

Incidentally, when cacheflush(2) was being used prior to it appearing in
the whitelist, I noticed that only a single thread was being killed by
SCMP_ACT_KILL (which the man page also confirms) rather than the whole
process, simply resulting in a lockup, and making it tricky to debug
since it wasn't immediately obvious what had happened (same thing can be
made to happen on x86 if e.g. read syscall is disallowed).

Should we be using the apparently more helpful SCMP_ACT_TRAP instead of
SCMP_ACT_KILL, or is that considered less secure? It would seem
preferable if we could kill the whole process in a recognisable way
instead of hanging it.

James Hogan (2):
  seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3
  configure: Enable seccomp sandbox for MIPS

 configure      | 3 +++
 qemu-seccomp.c | 4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
-- 
2.4.10

[Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[James Hogan]

The cacheflush system call (found on MIPS and ARM) has been included in
the libseccomp header since 2.2.0, so include include it back to that
version. Previously it was only enabled since 2.2.3 since that is when
it was enabled properly for ARM.

This will allow seccomp support to be enabled for MIPS back to
libseccomp 2.2.0.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 qemu-seccomp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 2866e3c2a660..9425dac17155 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -18,9 +18,7 @@
 
 #if SCMP_VER_MAJOR >= 3
   #define HAVE_CACHEFLUSH
-#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3
-  #define HAVE_CACHEFLUSH
-#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR == 2 && SCMP_VER_MICRO >= 3
+#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 2
   #define HAVE_CACHEFLUSH
 #endif
 
-- 
2.4.10

[Qemu-devel] [PATCH 2/2] configure: Enable seccomp sandbox for MIPS

[James Hogan]

Enable seccomp on MIPS since libseccomp version 2.2.0 when MIPS support
was first added.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
---
 configure | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configure b/configure
index 5db29f0245ae..f1c307bfc69c 100755
--- a/configure
+++ b/configure
@@ -1872,6 +1872,9 @@ if test "$seccomp" != "no" ; then
     i386|x86_64)
         libseccomp_minver="2.1.0"
         ;;
+    mips)
+        libseccomp_minver="2.2.0"
+        ;;
     arm|aarch64)
         libseccomp_minver="2.2.3"
         ;;
-- 
2.4.10

Re: [Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[Andrew Jones]

On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote:
> The cacheflush system call (found on MIPS and ARM) has been included in
> the libseccomp header since 2.2.0, so include include it back to that
> version. Previously it was only enabled since 2.2.3 since that is when
> it was enabled properly for ARM.
> 
> This will allow seccomp support to be enabled for MIPS back to
> libseccomp 2.2.0.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  qemu-seccomp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-By: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index 2866e3c2a660..9425dac17155 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -18,9 +18,7 @@
>  
>  #if SCMP_VER_MAJOR >= 3
>    #define HAVE_CACHEFLUSH
> -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3
> -  #define HAVE_CACHEFLUSH
> -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR == 2 && SCMP_VER_MICRO >= 3
> +#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 2
>    #define HAVE_CACHEFLUSH
>  #endif
>  
> -- 
> 2.4.10
> 
> 

Re: [Qemu-devel] [PATCH 2/2] configure: Enable seccomp sandbox for MIPS

[Andrew Jones]

On Mon, Apr 04, 2016 at 09:29:16AM +0100, James Hogan wrote:
> Enable seccomp on MIPS since libseccomp version 2.2.0 when MIPS support
> was first added.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  configure | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/configure b/configure
> index 5db29f0245ae..f1c307bfc69c 100755
> --- a/configure
> +++ b/configure
> @@ -1872,6 +1872,9 @@ if test "$seccomp" != "no" ; then
>      i386|x86_64)
>          libseccomp_minver="2.1.0"
>          ;;
> +    mips)
> +        libseccomp_minver="2.2.0"
> +        ;;
>      arm|aarch64)
>          libseccomp_minver="2.2.3"
>          ;;
> -- 
> 2.4.10
> 
>

Reviewed-By: Andrew Jones <drjones@redhat.com>

Now we just need an s390 person to test a 

 s390|s390x)
   libseccomp_minver="2.3.0" 

patch :-)

drew

Re: [Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[Peter Maydell]

On 8 April 2016 at 12:45, Andrew Jones <drjones@redhat.com> wrote:
> On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote:
>> The cacheflush system call (found on MIPS and ARM) has been included in
>> the libseccomp header since 2.2.0, so include include it back to that
>> version. Previously it was only enabled since 2.2.3 since that is when
>> it was enabled properly for ARM.
>>
>> This will allow seccomp support to be enabled for MIPS back to
>> libseccomp 2.2.0.
>>
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>>  qemu-seccomp.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> Reviewed-By: Andrew Jones <drjones@redhat.com>

Andrew: when we originally put this ifdeffery in you said
"libseccomp didn't know about cacheflush until 2.2.1, and we
 require 2.2.3 for other reasons"
(https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg07258.html)

Has something changed, were you just wrong back then, or have
you forgotten something this time around? (Do you remember what
the "other reasons" were?)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[Andrew Jones]

On Fri, Apr 08, 2016 at 12:54:54PM +0100, Peter Maydell wrote:
> On 8 April 2016 at 12:45, Andrew Jones <drjones@redhat.com> wrote:
> > On Mon, Apr 04, 2016 at 09:29:15AM +0100, James Hogan wrote:
> >> The cacheflush system call (found on MIPS and ARM) has been included in
> >> the libseccomp header since 2.2.0, so include include it back to that
> >> version. Previously it was only enabled since 2.2.3 since that is when
> >> it was enabled properly for ARM.
> >>
> >> This will allow seccomp support to be enabled for MIPS back to
> >> libseccomp 2.2.0.
> >>
> >> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> >> Cc: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> >> Cc: Aurelien Jarno <aurelien@aurel32.net>
> >> ---
> >>  qemu-seccomp.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > Reviewed-By: Andrew Jones <drjones@redhat.com>
> 
> Andrew: when we originally put this ifdeffery in you said
> "libseccomp didn't know about cacheflush until 2.2.1, and we
>  require 2.2.3 for other reasons"
> (https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg07258.html)

In that message I was referring to arm/aarch64 (the only arch at the
time that cared about cacheflush). 2.2.0 was the first version arm got
any cacheflush support (same version as x86 an mips), but it was
completely wrong until 2.2.1. Additional problems were then fixed for
2.2.3. x86 doesn't use cacheflush and mips didn't have any problems
(I guess), so they've been fine since 2.2.0.

As long as the configure script still requires a libseccomp_minver of
2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH
minimum version in qemu-seccomp.c is relaxed for the other
architectures that work.

> 
> Has something changed, were you just wrong back then, or have
> you forgotten something this time around? (Do you remember what
> the "other reasons" were?)

One reason was that the syscalls for ARM were assuming only a
__NR_ prefix instead of __ARM_NR_. And I think there was yet
another issue too, but I can't recall that one now.

Thanks,
drew

Re: [Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[Peter Maydell]

On 8 April 2016 at 13:39, Andrew Jones <drjones@redhat.com> wrote:
> In that message I was referring to arm/aarch64 (the only arch at the
> time that cared about cacheflush). 2.2.0 was the first version arm got
> any cacheflush support (same version as x86 an mips), but it was
> completely wrong until 2.2.1. Additional problems were then fixed for
> 2.2.3. x86 doesn't use cacheflush and mips didn't have any problems
> (I guess), so they've been fine since 2.2.0.
>
> As long as the configure script still requires a libseccomp_minver of
> 2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH
> minimum version in qemu-seccomp.c is relaxed for the other
> architectures that work.

OK. Could we have a comment, maybe something like:
 /* For some architectures (notably ARM) cacheflush is not
  * supported until libseccomp 2.2.3, but configure enforces that
  * we are using a more recent version on those hosts, so it is OK
  * for this check to be less strict.
  */

? (or feel free to reword if you have a better idea).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] seccomp: Whitelist cacheflush since 2.2.0 not 2.2.3

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On Fri, Apr 08, 2016 at 01:49:44PM +0100, Peter Maydell wrote:
> On 8 April 2016 at 13:39, Andrew Jones <drjones@redhat.com> wrote:
> > In that message I was referring to arm/aarch64 (the only arch at the
> > time that cared about cacheflush). 2.2.0 was the first version arm got
> > any cacheflush support (same version as x86 an mips), but it was
> > completely wrong until 2.2.1. Additional problems were then fixed for
> > 2.2.3. x86 doesn't use cacheflush and mips didn't have any problems
> > (I guess), so they've been fine since 2.2.0.
> >
> > As long as the configure script still requires a libseccomp_minver of
> > 2.2.3 for arm/aarch64, then it doesn't matter if the HAVE_CACHEFLUSH
> > minimum version in qemu-seccomp.c is relaxed for the other
> > architectures that work.
> 
> OK. Could we have a comment, maybe something like:
>  /* For some architectures (notably ARM) cacheflush is not
>   * supported until libseccomp 2.2.3, but configure enforces that
>   * we are using a more recent version on those hosts, so it is OK
>   * for this check to be less strict.
>   */
> 
> ? (or feel free to reword if you have a better idea).

Sure, I'll do a v2.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]