diff mbox

[v2] seccomp: add cacheflush to whitelist

Message ID 1446486994-29913-1-git-send-email-drjones@redhat.com
State New
Headers show

Commit Message

Andrew Jones Nov. 2, 2015, 5:56 p.m. UTC 
cacheflush is an arm-specific syscall that qemu built for arm
uses. Add it to the whitelist, but only if we're linking with
a recent enough libseccomp.

Signed-off-by: Andrew Jones <drjones@redhat.com>

---
v2: only add cacheflush if libseccomp supports it

 qemu-seccomp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
1.8.3.1

Comments

Peter Maydell Nov. 2, 2015, 6:09 p.m. UTC | #1 
On 2 November 2015 at 17:56, Andrew Jones <drjones@redhat.com> wrote:
> cacheflush is an arm-specific syscall that qemu built for arm

> uses. Add it to the whitelist, but only if we're linking with

> a recent enough libseccomp.

>

> Signed-off-by: Andrew Jones <drjones@redhat.com>

> ---

> v2: only add cacheflush if libseccomp supports it

>

>  qemu-seccomp.c | 9 ++++++++-

>  1 file changed, 8 insertions(+), 1 deletion(-)

>

> diff --git a/qemu-seccomp.c b/qemu-seccomp.c

> index 80d034a8d5190..e76097e958779 100644

> --- a/qemu-seccomp.c

> +++ b/qemu-seccomp.c

> @@ -16,6 +16,10 @@

>  #include <seccomp.h>

>  #include "sysemu/seccomp.h"

>

> +#if SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2 && SCMP_VER_MICRO >= 3

> +#define HAVE_CACHEFLUSH

> +#endif


This will claim that a hypothetical future version 3.0.0 does not
have cacheflush...

thanks
-- PMM
Andrew Jones Nov. 2, 2015, 7:04 p.m. UTC | #2 
On Mon, Nov 02, 2015 at 06:09:41PM +0000, Peter Maydell wrote:
> On 2 November 2015 at 17:56, Andrew Jones <drjones@redhat.com> wrote:

> > cacheflush is an arm-specific syscall that qemu built for arm

> > uses. Add it to the whitelist, but only if we're linking with

> > a recent enough libseccomp.

> >

> > Signed-off-by: Andrew Jones <drjones@redhat.com>

> > ---

> > v2: only add cacheflush if libseccomp supports it

> >

> >  qemu-seccomp.c | 9 ++++++++-

> >  1 file changed, 8 insertions(+), 1 deletion(-)

> >

> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c

> > index 80d034a8d5190..e76097e958779 100644

> > --- a/qemu-seccomp.c

> > +++ b/qemu-seccomp.c

> > @@ -16,6 +16,10 @@

> >  #include <seccomp.h>

> >  #include "sysemu/seccomp.h"

> >

> > +#if SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2 && SCMP_VER_MICRO >= 3

> > +#define HAVE_CACHEFLUSH

> > +#endif

> 

> This will claim that a hypothetical future version 3.0.0 does not

> have cacheflush...


Indeed. Sigh... In that case, how about just

#if defined(TARGET_ARM) || defined(TARGET_AARCH64)
    { SCMP_SYS(cacheflush), 240 },
#endif

Thanks,
drew

> 

> thanks

> -- PMM

>
Peter Maydell Nov. 2, 2015, 8:37 p.m. UTC | #3 
On 2 November 2015 at 19:04, Andrew Jones <drjones@redhat.com> wrote:
> On Mon, Nov 02, 2015 at 06:09:41PM +0000, Peter Maydell wrote:

>> On 2 November 2015 at 17:56, Andrew Jones <drjones@redhat.com> wrote:

>> > cacheflush is an arm-specific syscall that qemu built for arm

>> > uses. Add it to the whitelist, but only if we're linking with

>> > a recent enough libseccomp.

>> >

>> > Signed-off-by: Andrew Jones <drjones@redhat.com>

>> > ---

>> > v2: only add cacheflush if libseccomp supports it

>> >

>> >  qemu-seccomp.c | 9 ++++++++-

>> >  1 file changed, 8 insertions(+), 1 deletion(-)

>> >

>> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c

>> > index 80d034a8d5190..e76097e958779 100644

>> > --- a/qemu-seccomp.c

>> > +++ b/qemu-seccomp.c

>> > @@ -16,6 +16,10 @@

>> >  #include <seccomp.h>

>> >  #include "sysemu/seccomp.h"

>> >

>> > +#if SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2 && SCMP_VER_MICRO >= 3

>> > +#define HAVE_CACHEFLUSH

>> > +#endif

>>

>> This will claim that a hypothetical future version 3.0.0 does not

>> have cacheflush...

>

> Indeed. Sigh... In that case, how about just

>

> #if defined(TARGET_ARM) || defined(TARGET_AARCH64)

>     { SCMP_SYS(cacheflush), 240 },

> #endif


You want to be checking based on the host architecture,
not the target architecture. Also, not doing the check based
on seccomp version means there's no hint in the code that the
ifdefs become obsolete if we raise our cross-architecture
minimum seccomp version requirement in the future, so really
a version check is better I think.

thanks
-- PMM
Andrew Jones Nov. 2, 2015, 10:18 p.m. UTC | #4 
On Mon, Nov 02, 2015 at 08:37:15PM +0000, Peter Maydell wrote:
> On 2 November 2015 at 19:04, Andrew Jones <drjones@redhat.com> wrote:

> > On Mon, Nov 02, 2015 at 06:09:41PM +0000, Peter Maydell wrote:

> >> On 2 November 2015 at 17:56, Andrew Jones <drjones@redhat.com> wrote:

> >> > cacheflush is an arm-specific syscall that qemu built for arm

> >> > uses. Add it to the whitelist, but only if we're linking with

> >> > a recent enough libseccomp.

> >> >

> >> > Signed-off-by: Andrew Jones <drjones@redhat.com>

> >> > ---

> >> > v2: only add cacheflush if libseccomp supports it

> >> >

> >> >  qemu-seccomp.c | 9 ++++++++-

> >> >  1 file changed, 8 insertions(+), 1 deletion(-)

> >> >

> >> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c

> >> > index 80d034a8d5190..e76097e958779 100644

> >> > --- a/qemu-seccomp.c

> >> > +++ b/qemu-seccomp.c

> >> > @@ -16,6 +16,10 @@

> >> >  #include <seccomp.h>

> >> >  #include "sysemu/seccomp.h"

> >> >

> >> > +#if SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2 && SCMP_VER_MICRO >= 3

> >> > +#define HAVE_CACHEFLUSH

> >> > +#endif

> >>

> >> This will claim that a hypothetical future version 3.0.0 does not

> >> have cacheflush...

> >

> > Indeed. Sigh... In that case, how about just

> >

> > #if defined(TARGET_ARM) || defined(TARGET_AARCH64)

> >     { SCMP_SYS(cacheflush), 240 },

> > #endif

> 

> You want to be checking based on the host architecture,

> not the target architecture. Also, not doing the check based

> on seccomp version means there's no hint in the code that the

> ifdefs become obsolete if we raise our cross-architecture

> minimum seccomp version requirement in the future, so really

> a version check is better I think.


Right. OK, I'll stop flinging junk and pull a better version
check together.

Thanks,
drew

> 

> thanks

> -- PMM

>
diff mbox

Patch

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 80d034a8d5190..e76097e958779 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -16,6 +16,10 @@ 
 #include <seccomp.h>
 #include "sysemu/seccomp.h"
 
+#if SCMP_VER_MAJOR >= 2 && SCMP_VER_MINOR >= 2 && SCMP_VER_MICRO >= 3
+#define HAVE_CACHEFLUSH
+#endif
+
 struct QemuSeccompSyscall {
     int32_t num;
     uint8_t priority;
@@ -238,7 +242,10 @@  static const struct QemuSeccompSyscall seccomp_whitelist[] = {
     { SCMP_SYS(inotify_init1), 240 },
     { SCMP_SYS(inotify_add_watch), 240 },
     { SCMP_SYS(mbind), 240 },
-    { SCMP_SYS(memfd_create), 240 }
+    { SCMP_SYS(memfd_create), 240 },
+#ifdef HAVE_CACHEFLUSH
+    { SCMP_SYS(cacheflush), 240 },
+#endif
 };
 
 int seccomp_start(void)