[CRIU] [PATCH 1/2] don't assume the kernel has CONFIG_SECCOMP

Tycho Andersen tycho.andersen at canonical.com
Fri Jun 26 15:45:35 PDT 2015


On Sat, Jun 27, 2015 at 01:25:41AM +0300, Andrey Wagin wrote:
> 2015-06-27 1:06 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:
> > linux/seccomp.h may not be available, and the seccomp mode might not be
> > listed in /proc/pid/status, so let's not assume those two things are
> > present.
> >
> > v2: add a seccomp.h with all the constants we use from linux/seccomp.h
> > v3: don't do a compile time check for PTRACE_O_SUSPEND_SECCOMP, just let
> >     ptrace return EINVAL for it; also add a checkskip to skip the
> >     seccomp_strict test if PTRACE_O_SUSPEND_SECCOMP or linux/seccomp.h
> >     aren't present.
> >
> > Reported-by: Mr. Jenkins
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> >  cr-dump.c                                      |  3 +--
> >  cr-restore.c                                   |  3 +--
> >  include/seccomp.h                              | 16 ++++++++++++++++
> >  pie/restorer.c                                 | 13 +------------
> >  proc_parse.c                                   |  4 ++--
> >  ptrace.c                                       | 11 +----------
> >  scripts/feature-tests.mak                      | 11 -----------
> >  test/zdtm/live/static/seccomp_strict.checkskip | 18 ++++++++++++++++++
> >  8 files changed, 40 insertions(+), 39 deletions(-)
> >  create mode 100644 include/seccomp.h
> >  create mode 100755 test/zdtm/live/static/seccomp_strict.checkskip
> >
> > diff --git a/cr-dump.c b/cr-dump.c
> > index 8936a64..9505f5e 100644
> > --- a/cr-dump.c
> > +++ b/cr-dump.c
> > @@ -19,8 +19,6 @@
> >  #include <sched.h>
> >  #include <sys/resource.h>
> >
> > -#include <linux/seccomp.h>
> > -
> >  #include "protobuf.h"
> >  #include "protobuf/fdinfo.pb-c.h"
> >  #include "protobuf/fs.pb-c.h"
> > @@ -77,6 +75,7 @@
> >  #include "aio.h"
> >  #include "security.h"
> >  #include "lsm.h"
> > +#include "seccomp.h"
> >
> >  #include "asm/dump.h"
> >
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 45c746e..7439a05 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -24,8 +24,6 @@
> >
> >  #include <sys/sendfile.h>
> >
> > -#include <linux/seccomp.h>
> > -
> >  #include "ptrace.h"
> >  #include "compiler.h"
> >  #include "asm/types.h"
> > @@ -77,6 +75,7 @@
> >  #include "aio.h"
> >  #include "security.h"
> >  #include "lsm.h"
> > +#include "seccomp.h"
> >
> >  #include "parasite-syscall.h"
> >
> > diff --git a/include/seccomp.h b/include/seccomp.h
> > new file mode 100644
> > index 0000000..017dcd4
> > --- /dev/null
> > +++ b/include/seccomp.h
> > @@ -0,0 +1,16 @@
> > +#ifndef __CR_SECCOMP_H__
> > +#define __CR_SECCOMP_H__
> > +
> > +#ifndef SECCOMP_MODE_DISABLED
> > +#define SECCOMP_MODE_DISABLED 0
> > +#endif
> > +
> > +#ifndef SECCOMP_MODE_STRICT
> > +#define SECCOMP_MODE_STRICT 1
> > +#endif
> > +
> > +#ifndef SECCOMP_MODE_FILTER
> > +#define SECCOMP_MODE_FILTER 2
> > +#endif
> > +
> > +#endif
> > diff --git a/pie/restorer.c b/pie/restorer.c
> > index 4150b49..8c6b421 100644
> > --- a/pie/restorer.c
> > +++ b/pie/restorer.c
> > @@ -30,6 +30,7 @@
> >  #include "lock.h"
> >  #include "restorer.h"
> >  #include "aio.h"
> > +#include "seccomp.h"
> >
> >  #include "protobuf/creds.pb-c.h"
> >  #include "protobuf/mm.pb-c.h"
> > @@ -40,18 +41,6 @@
> >  #define PR_SET_PDEATHSIG 1
> >  #endif
> >
> > -#ifndef SECCOMP_MODE_DISABLED
> > -#define SECCOMP_MODE_DISABLED 0
> > -#endif
> > -
> > -#ifndef SECCOMP_MODE_STRICT
> > -#define SECCOMP_MODE_STRICT 1
> > -#endif
> > -
> > -#ifndef SECCOMP_MODE_FILTER
> > -#define SECCOMP_MODE_FILTER 2
> > -#endif
> > -
> >  #define sys_prctl_safe(opcode, val1, val2, val3)                       \
> >         ({                                                              \
> >                 long __ret = sys_prctl(opcode, val1, val2, val3, 0);    \
> > diff --git a/proc_parse.c b/proc_parse.c
> > index 168afcb..e940fc1 100644
> > --- a/proc_parse.c
> > +++ b/proc_parse.c
> > @@ -9,7 +9,6 @@
> >  #include <string.h>
> >  #include <ctype.h>
> >  #include <linux/fs.h>
> > -#include <linux/seccomp.h>
> >
> >  #include "asm/types.h"
> >  #include "list.h"
> > @@ -28,6 +27,7 @@
> >  #include "proc_parse.h"
> >  #include "cr_options.h"
> >  #include "sysfs_parse.h"
> > +#include "seccomp.h"
> >  #include "protobuf.h"
> >  #include "protobuf/fdinfo.pb-c.h"
> >  #include "protobuf/mnt.pb-c.h"
> > @@ -856,7 +856,7 @@ int parse_pid_status(pid_t pid, struct proc_status_creds *cr)
> >                 }
> >         }
> >
> > -       if (done == 9)
> > +       if (done >= 8)
> >                 ret = 0;
> >
> >  err_parse:
> > diff --git a/ptrace.c b/ptrace.c
> > index 4f9e66e..c3a9553 100644
> > --- a/ptrace.c
> > +++ b/ptrace.c
> > @@ -14,8 +14,6 @@
> >  #include <sys/resource.h>
> >  #include <sys/wait.h>
> >
> > -#include <linux/seccomp.h>
> > -
> >  #include "compiler.h"
> >  #include "asm/types.h"
> >  #include "util.h"
> > @@ -23,6 +21,7 @@
> >  #include "proc_parse.h"
> >  #include "crtools.h"
> >  #include "security.h"
> > +#include "seccomp.h"
> >
> >  int unseize_task(pid_t pid, int orig_st, int st)
> >  {
> > @@ -41,7 +40,6 @@ int unseize_task(pid_t pid, int orig_st, int st)
> >         return ptrace(PTRACE_DETACH, pid, NULL, NULL);
> >  }
> >
> > -#ifdef CONFIG_HAS_SUSPEND_SECCOMP
> >  int suspend_seccomp(pid_t pid)
> >  {
> >         if (ptrace(PTRACE_SETOPTIONS, pid, NULL, PTRACE_O_SUSPEND_SECCOMP) < 0) {
> > @@ -51,13 +49,6 @@ int suspend_seccomp(pid_t pid)
> >
> >         return 0;
> >  }
> > -#else
> > -int suspend_seccomp(pid_t pid)
> > -{
> > -       pr_err("seccomp enabled and seccomp suspending not supported\n");
> > -       return -1;
> > -}
> > -#endif
> >
> >  /*
> >   * This routine seizes task putting it into a special
> > diff --git a/scripts/feature-tests.mak b/scripts/feature-tests.mak
> > index ec7972a..519eb52 100644
> > --- a/scripts/feature-tests.mak
> > +++ b/scripts/feature-tests.mak
> > @@ -92,14 +92,3 @@ int main(int argc, char *argv[], char *envp[])
> >  }
> >
> >  endef
> > -
> > -define PTRACE_SUSPEND_SECCOMP_TEST
> > -
> > -#include <linux/ptrace.h>
> > -
> > -int main(void)
> > -{
> > -       return PTRACE_O_SUSPEND_SECCOMP;
> > -}
> > -
> > -endef
> > diff --git a/test/zdtm/live/static/seccomp_strict.checkskip b/test/zdtm/live/static/seccomp_strict.checkskip
> > new file mode 100755
> > index 0000000..615bbd2
> > --- /dev/null
> > +++ b/test/zdtm/live/static/seccomp_strict.checkskip
> > @@ -0,0 +1,18 @@
> > +#!/bin/bash
> > +
> > +cat > seccomp_header_test.c << EOF
> > +#include <linux/seccomp.h>
> > +#include <linux/ptrace.h>
> > +
> > +int main(void)
> > +{
> > +  return PTRACE_O_SUSPEND_SECCOMP;
> 
> This doesn't mean that PTRACE_O_SUSPEND_SECCOMP is supported by the
> current kernel.

It doesn't?

> We usually use "criu check --feature ..." to determine
> whether a feature is supported. When I said that we need to fix tests,
> I mean that we need to be able to compile them if PR_SET_SECCOMP and
> SECCOMP_MODE_STRICT are not defined in global headers.

Ok, but the test will fail on kernels that don't support it, so isn't
it better to skip it all together if it's not supported? I could
switch the compile above to be a criu check --feature instead.

Tycho

> > +}
> > +EOF
> > +
> > +gcc -o seccomp_header_test seccomp_header_test.c
> > +ret=$?
> > +
> > +rm seccomp_header_test*
> > +
> > +exit $ret
> > --
> > 2.1.4
> >


More information about the CRIU mailing list