[CRIU] [PATCH 1/2] don't assume the kernel has CONFIG_SECCOMP
Tycho Andersen
tycho.andersen at canonical.com
Mon Jun 29 07:11:32 PDT 2015
On Sat, Jun 27, 2015 at 07:56:14AM +0300, Andrey Wagin wrote:
> 2015-06-27 1:45 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:
> > 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?
>
> The host can have old headers and a new kernel. For example:
>
> [root at avagin-fc19-cr ~]# uname -a
> Linux avagin-fc19-cr 4.1.0-rc8-next-20150617 #248 SMP Fri Jun 19
> 23:09:08 MSK 2015 x86_64 x86_64 x86_64 GNU/Linux
>
> [root at avagin-fc19-cr ~]# rpm -qf /usr/include/linux/ptrace.h
> kernel-headers-3.18.9-200.fc21.x86_64
>
> I don't install headers for each kernel and I think many of us do the same.
Ah, ok, I didn't realize people did this.
> >
> >> 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.
>
> Here are two points.
> * When we need to execute a test
> "criu check --feature" is called from zdtm.sh and it's used to
> determine whether we need to execute a test. You can look into zdtm.sh
> how it's used for other features.
>
> '''
> $CRIU check -v0 --feature "tun"
> if [ $? -eq 0 ]; then
> TEST_LIST="$TEST_LIST$TEST_TUN"
> fi
> '''
Ok, I can change it to work like this.
> * How to compile a test if global headers doesn't contain required constants.
> We need to define PR_SET_SECCOMP and SECCOMP_MODE_STRICT to be able to
According to my man page, PR_SET_SECCOMP was added in 2.6.23, so it
should be ok just getting it from the kernel headers IIUC.
Tycho
> compile the test. This part is done in my version of this patch.
>
> >
> > 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