[CRIU] [PATCH 1/2] don't assume the kernel has CONFIG_SECCOMP
Andrey Wagin
avagin at gmail.com
Fri Jun 26 21:56:14 PDT 2015
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.
>
>> 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
'''
* 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
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