Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1707759 > unrolled thread

[PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS

Started byKees Cook <keescook@chromium.org>
First post2017-08-09 21:10 +0200
Last post2017-08-09 23:00 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Kees Cook <keescook@chromium.org> - 2017-08-09 21:10 +0200
    [PATCH v3 4/4] selftests/seccomp: Test thread vs process killing Kees Cook <keescook@chromium.org> - 2017-08-09 21:10 +0200
    [PATCH v3 1/4] seccomp: Provide matching filter for introspection Kees Cook <keescook@chromium.org> - 2017-08-09 21:10 +0200
    Re: [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Tycho Andersen <tycho@docker.com> - 2017-08-09 22:30 +0200
      Re: [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Tyler Hicks <tyhicks@canonical.com> - 2017-08-09 22:40 +0200
        Re: [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Tycho Andersen <tycho@docker.com> - 2017-08-09 22:50 +0200
        Re: [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS Kees Cook <keescook@chromium.org> - 2017-08-09 23:00 +0200

#1707759 — [PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS

FromKees Cook <keescook@chromium.org>
Date2017-08-09 21:10 +0200
Subject[PATCH v3 0/4] seccomp: Add SECCOMP_FILTER_FLAG_KILL_PROCESS
Message-ID<ucEwF-30X-5@gated-at.bofh.it>
This series is the result of Fabricio and I going around a few times
on possible solutions for finding a way to enhance RET_KILL to kill
the process group. There's a lot of ways this could be done, but I
wanted something that felt cleanest. As it happens, Tyler's recent
patch series for logging improvement also needs to know a litte bit
more during filter runs, and the solution for both is to pass back
the matched filter. This lets us examine it here for RET_KILL and
in the future for logging changes.

The filter passing is patch 1, the new flag for RET_KILL is patch 2.
Some test refactoring is in patch 3 for the RET_DATA ordering, and
patch 4 is the test for the new RET_KILL flag.

One thing missing is that CRIU will likely need to be updated, since
saving/restoring seccomp filter _rules_ will not include the filter
_flags_ for a process. This can be addressed separately.

Please take a look!

Thanks,

-Kees

v3:
- adjust seccomp_run_filters() to avoid later filters from masking
  kill-process RET_KILL actions (drewry)
- add test for masked RET_KILL.

v2:
- moved kill_process bool into struct padding gap (tyhicks)
- improved comments/docs in various places for clarify (tyhicks)
- use ASSERT_TRUE() for WIFEXITED and WIFSIGNALLED (tyhicks)
- adding Reviewed-bys from tyhicks

[toc] | [next] | [standalone]


#1707760 — [PATCH v3 4/4] selftests/seccomp: Test thread vs process killing

FromKees Cook <keescook@chromium.org>
Date2017-08-09 21:10 +0200
Subject[PATCH v3 4/4] selftests/seccomp: Test thread vs process killing
Message-ID<ucEwF-30X-13@gated-at.bofh.it>
In reply to#1707759
SECCOMP_RET_KILL is supposed to kill the current thread (and userspace
depends on this), so test for this, distinct from killing the entire
process. This also tests killing the entire process with the new
SECCOMP_FILTER_FLAG_KILL_PROCESS flag. (This also moves a bunch of
defines up earlier in the file to use them earlier.)

Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 192 ++++++++++++++++++++------
 1 file changed, 151 insertions(+), 41 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index ee78a53da5d1..92f2779b6309 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -87,6 +87,51 @@ struct seccomp_data {
 };
 #endif
 
+#ifndef __NR_seccomp
+# if defined(__i386__)
+#  define __NR_seccomp 354
+# elif defined(__x86_64__)
+#  define __NR_seccomp 317
+# elif defined(__arm__)
+#  define __NR_seccomp 383
+# elif defined(__aarch64__)
+#  define __NR_seccomp 277
+# elif defined(__hppa__)
+#  define __NR_seccomp 338
+# elif defined(__powerpc__)
+#  define __NR_seccomp 358
+# elif defined(__s390__)
+#  define __NR_seccomp 348
+# else
+#  warning "seccomp syscall number unknown for this architecture"
+#  define __NR_seccomp 0xffff
+# endif
+#endif
+
+#ifndef SECCOMP_SET_MODE_STRICT
+#define SECCOMP_SET_MODE_STRICT 0
+#endif
+
+#ifndef SECCOMP_SET_MODE_FILTER
+#define SECCOMP_SET_MODE_FILTER 1
+#endif
+
+#ifndef SECCOMP_FILTER_FLAG_TSYNC
+#define SECCOMP_FILTER_FLAG_TSYNC 1
+#endif
+
+#ifndef SECCOMP_FILTER_FLAG_KILL_PROCESS
+#define SECCOMP_FILTER_FLAG_KILL_PROCESS 2
+#endif
+
+#ifndef seccomp
+int seccomp(unsigned int op, unsigned int flags, void *args)
+{
+	errno = 0;
+	return syscall(__NR_seccomp, op, flags, args);
+}
+#endif
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define syscall_arg(_n) (offsetof(struct seccomp_data, args[_n]))
 #elif __BYTE_ORDER == __BIG_ENDIAN
@@ -520,6 +565,112 @@ TEST_SIGNAL(KILL_one_arg_six, SIGSYS)
 	close(fd);
 }
 
+/* This is a thread task to die via seccomp filter violation. */
+void *kill_thread(void *data)
+{
+	bool die = (bool)data;
+
+	if (die) {
+		prctl(PR_GET_SECCOMP, 0, 0, 0, 0);
+		return (void *)SIBLING_EXIT_FAILURE;
+	}
+
+	return (void *)SIBLING_EXIT_UNKILLED;
+}
+
+/* Prepare a thread that will kill itself or both of us. */
+void kill_thread_or_group(struct __test_metadata *_metadata, bool kill_process)
+{
+	pthread_t thread;
+	void *status;
+	unsigned int flags;
+	/* Kill only when calling __NR_prctl. */
+	struct sock_filter filter[] = {
+		BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+			offsetof(struct seccomp_data, nr)),
+		BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_prctl, 0, 1),
+		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+	};
+	struct sock_fprog prog = {
+		.len = (unsigned short)ARRAY_SIZE(filter),
+		.filter = filter,
+	};
+
+	ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
+		TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!");
+	}
+
+	flags = kill_process ? SECCOMP_FILTER_FLAG_KILL_PROCESS : 0;
+	ASSERT_EQ(0, seccomp(SECCOMP_SET_MODE_FILTER, flags, &prog)) {
+		if (kill_process)
+			TH_LOG("Kernel does not support SECCOMP_FILTER_FLAG_KILL_PROCESS");
+		else
+			TH_LOG("Kernel does not support seccomp syscall");
+	}
+
+	/*
+	 * Add the kill rule again to make sure that the KILL_PROCESS
+	 * flag cannot be downgraded by a new filter.
+	 */
+	ASSERT_EQ(0, seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog));
+
+	/* Start a thread that will exit immediately. */
+	ASSERT_EQ(0, pthread_create(&thread, NULL, kill_thread, (void *)false));
+	ASSERT_EQ(0, pthread_join(thread, &status));
+	ASSERT_EQ(SIBLING_EXIT_UNKILLED, (unsigned long)status);
+
+	/* Start a thread that will die immediately. */
+	ASSERT_EQ(0, pthread_create(&thread, NULL, kill_thread, (void *)true));
+	ASSERT_EQ(0, pthread_join(thread, &status));
+	ASSERT_NE(SIBLING_EXIT_FAILURE, (unsigned long)status);
+
+	/*
+	 * If we get here, only the spawned thread died. Let the parent know
+	 * the whole process didn't die (i.e. this thread, the spawner,
+	 * stayed running).
+	 */
+	exit(42);
+}
+
+TEST(KILL_thread)
+{
+	int status;
+	pid_t child_pid;
+
+	child_pid = fork();
+	ASSERT_LE(0, child_pid);
+	if (child_pid == 0) {
+		kill_thread_or_group(_metadata, false);
+		_exit(38);
+	}
+
+	ASSERT_EQ(child_pid, waitpid(child_pid, &status, 0));
+
+	/* If only the thread was killed, we'll see exit 42. */
+	ASSERT_TRUE(WIFEXITED(status));
+	ASSERT_EQ(42, WEXITSTATUS(status));
+}
+
+TEST(KILL_process)
+{
+	int status;
+	pid_t child_pid;
+
+	child_pid = fork();
+	ASSERT_LE(0, child_pid);
+	if (child_pid == 0) {
+		kill_thread_or_group(_metadata, true);
+		_exit(38);
+	}
+
+	ASSERT_EQ(child_pid, waitpid(child_pid, &status, 0));
+
+	/* If the entire process was killed, we'll see SIGSYS. */
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_EQ(SIGSYS, WTERMSIG(status));
+}
+
 /* TODO(wad) add 64-bit versus 32-bit arg tests. */
 TEST(arg_out_of_range)
 {
@@ -1675,47 +1826,6 @@ TEST_F_SIGNAL(TRACE_syscall, kill_after_ptrace, SIGSYS)
 	EXPECT_NE(self->mypid, syscall(__NR_getpid));
 }
 
-#ifndef __NR_seccomp
-# if defined(__i386__)
-#  define __NR_seccomp 354
-# elif defined(__x86_64__)
-#  define __NR_seccomp 317
-# elif defined(__arm__)
-#  define __NR_seccomp 383
-# elif defined(__aarch64__)
-#  define __NR_seccomp 277
-# elif defined(__hppa__)
-#  define __NR_seccomp 338
-# elif defined(__powerpc__)
-#  define __NR_seccomp 358
-# elif defined(__s390__)
-#  define __NR_seccomp 348
-# else
-#  warning "seccomp syscall number unknown for this architecture"
-#  define __NR_seccomp 0xffff
-# endif
-#endif
-
-#ifndef SECCOMP_SET_MODE_STRICT
-#define SECCOMP_SET_MODE_STRICT 0
-#endif
-
-#ifndef SECCOMP_SET_MODE_FILTER
-#define SECCOMP_SET_MODE_FILTER 1
-#endif
-
-#ifndef SECCOMP_FILTER_FLAG_TSYNC
-#define SECCOMP_FILTER_FLAG_TSYNC 1
-#endif
-
-#ifndef seccomp
-int seccomp(unsigned int op, unsigned int flags, void *args)
-{
-	errno = 0;
-	return syscall(__NR_seccomp, op, flags, args);
-}
-#endif
-
 TEST(seccomp_syscall)
 {
 	struct sock_filter filter[] = {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1707762 — [PATCH v3 1/4] seccomp: Provide matching filter for introspection

FromKees Cook <keescook@chromium.org>
Date2017-08-09 21:10 +0200
Subject[PATCH v3 1/4] seccomp: Provide matching filter for introspection
Message-ID<ucEwG-30X-21@gated-at.bofh.it>
In reply to#1707759
Both the upcoming logging improvements and changes to RET_KILL will need
to know which filter a given seccomp return value originated from. In
order to delay logic processing of result until after the seccomp loop,
this adds a single pointer assignment on matches. This will allow both
log and RET_KILL logic to work off the filter rather than doing more
expensive tests inside the time-critical run_filters loop.

Running tight cycles of getpid() with filters attached shows no measurable
difference in speed.

Suggested-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
---
 kernel/seccomp.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 98b59b5db90b..1f3347fc2605 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -171,10 +171,14 @@ static int seccomp_check_filter(struct sock_filter *filter, unsigned int flen)
 /**
  * seccomp_run_filters - evaluates all seccomp filters against @sd
  * @sd: optional seccomp data to be passed to filters
+ * @match: stores struct seccomp_filter that resulted in the return value,
+ *         unless filter returned SECCOMP_RET_ALLOW, in which case it will
+ *         be unchanged.
  *
  * Returns valid seccomp BPF response codes.
  */
-static u32 seccomp_run_filters(const struct seccomp_data *sd)
+static u32 seccomp_run_filters(const struct seccomp_data *sd,
+			       struct seccomp_filter **match)
 {
 	struct seccomp_data sd_local;
 	u32 ret = SECCOMP_RET_ALLOW;
@@ -198,8 +202,10 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd)
 	for (; f; f = f->prev) {
 		u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
 
-		if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION))
+		if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION)) {
 			ret = cur_ret;
+			*match = f;
+		}
 	}
 	return ret;
 }
@@ -566,6 +572,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
 			    const bool recheck_after_trace)
 {
 	u32 filter_ret, action;
+	struct seccomp_filter *match = NULL;
 	int data;
 
 	/*
@@ -574,7 +581,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
 	 */
 	rmb();
 
-	filter_ret = seccomp_run_filters(sd);
+	filter_ret = seccomp_run_filters(sd, &match);
 	data = filter_ret & SECCOMP_RET_DATA;
 	action = filter_ret & SECCOMP_RET_ACTION;
 
@@ -638,6 +645,11 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
 		return 0;
 
 	case SECCOMP_RET_ALLOW:
+		/*
+		 * Note that the "match" filter will always be NULL for
+		 * this action since SECCOMP_RET_ALLOW is the starting
+		 * state in seccomp_run_filters().
+		 */
 		return 0;
 
 	case SECCOMP_RET_KILL:
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1707846

FromTycho Andersen <tycho@docker.com>
Date2017-08-09 22:30 +0200
Message-ID<ucFM6-3O1-15@gated-at.bofh.it>
In reply to#1707759
On Wed, Aug 09, 2017 at 12:01:53PM -0700, Kees Cook wrote:
> This series is the result of Fabricio and I going around a few times
> on possible solutions for finding a way to enhance RET_KILL to kill
> the process group. There's a lot of ways this could be done, but I
> wanted something that felt cleanest. As it happens, Tyler's recent
> patch series for logging improvement also needs to know a litte bit
> more during filter runs, and the solution for both is to pass back
> the matched filter. This lets us examine it here for RET_KILL and
> in the future for logging changes.
> 
> The filter passing is patch 1, the new flag for RET_KILL is patch 2.
> Some test refactoring is in patch 3 for the RET_DATA ordering, and
> patch 4 is the test for the new RET_KILL flag.
> 
> One thing missing is that CRIU will likely need to be updated, since
> saving/restoring seccomp filter _rules_ will not include the filter
> _flags_ for a process. This can be addressed separately.

Thanks for the heads up, I suppose PTRACE_SECCOMP_GET_FLAGS similar to
how PTRACE_SECCOMP_GET_FILTER works will be fine for this. One
question is: would we then also need to keep track of the TSYNC flag?
I don't think CRIU needs this to be correct, and we can grab the
KILL_PROCESS flag from filter->kill_process, so perhaps it's moot.

Anyway, happy to do this and the userspace part when this lands.

Cheers,

Tycho

> Please take a look!
> 
> Thanks,
> 
> -Kees
> 
> v3:
> - adjust seccomp_run_filters() to avoid later filters from masking
>   kill-process RET_KILL actions (drewry)
> - add test for masked RET_KILL.
> 
> v2:
> - moved kill_process bool into struct padding gap (tyhicks)
> - improved comments/docs in various places for clarify (tyhicks)
> - use ASSERT_TRUE() for WIFEXITED and WIFSIGNALLED (tyhicks)
> - adding Reviewed-bys from tyhicks
> 

[toc] | [prev] | [next] | [standalone]


#1707858

FromTyler Hicks <tyhicks@canonical.com>
Date2017-08-09 22:40 +0200
Message-ID<ucFVN-3Rs-27@gated-at.bofh.it>
In reply to#1707846

[Multipart message — attachments visible in raw view] — view raw

Hey Tycho!

On 08/09/2017 03:22 PM, Tycho Andersen wrote:
> On Wed, Aug 09, 2017 at 12:01:53PM -0700, Kees Cook wrote:
>> This series is the result of Fabricio and I going around a few times
>> on possible solutions for finding a way to enhance RET_KILL to kill
>> the process group. There's a lot of ways this could be done, but I
>> wanted something that felt cleanest. As it happens, Tyler's recent
>> patch series for logging improvement also needs to know a litte bit
>> more during filter runs, and the solution for both is to pass back
>> the matched filter. This lets us examine it here for RET_KILL and
>> in the future for logging changes.
>>
>> The filter passing is patch 1, the new flag for RET_KILL is patch 2.
>> Some test refactoring is in patch 3 for the RET_DATA ordering, and
>> patch 4 is the test for the new RET_KILL flag.
>>
>> One thing missing is that CRIU will likely need to be updated, since
>> saving/restoring seccomp filter _rules_ will not include the filter
>> _flags_ for a process. This can be addressed separately.
> 
> Thanks for the heads up, I suppose PTRACE_SECCOMP_GET_FLAGS similar to
> how PTRACE_SECCOMP_GET_FILTER works will be fine for this. One
> question is: would we then also need to keep track of the TSYNC flag?
> I don't think CRIU needs this to be correct, and we can grab the
> KILL_PROCESS flag from filter->kill_process, so perhaps it's moot.

Note that the logging changes that I'm working on also introduce a new
filter flag (as Kees mentioned above). My filter flag is a lot like the
KILL_PROCESS filter flag in that it is stored as a member of the
seccomp_filter struct.

I would think that you'd want to be able to do something like
PTRACE_SECCOMP_GET_FILTER to (hopefully) future proof CRIU against all
newly added filter flags.

I'll also mention that I have a libseccomp branch in the making that
allows libseccomp to query the kernel to see if it supports a given
filter flag. I haven't done a PR on that yet because I'm waiting to see
how my related kernel patches play out (they seem to be getting close to
being acceptable).

Tyler

> 
> Anyway, happy to do this and the userspace part when this lands.
> 
> Cheers,
> 
> Tycho
> 
>> Please take a look!
>>
>> Thanks,
>>
>> -Kees
>>
>> v3:
>> - adjust seccomp_run_filters() to avoid later filters from masking
>>   kill-process RET_KILL actions (drewry)
>> - add test for masked RET_KILL.
>>
>> v2:
>> - moved kill_process bool into struct padding gap (tyhicks)
>> - improved comments/docs in various places for clarify (tyhicks)
>> - use ASSERT_TRUE() for WIFEXITED and WIFSIGNALLED (tyhicks)
>> - adding Reviewed-bys from tyhicks
>>


[toc] | [prev] | [next] | [standalone]


#1707927

FromTycho Andersen <tycho@docker.com>
Date2017-08-09 22:50 +0200
Message-ID<ucG5x-3Vm-115@gated-at.bofh.it>
In reply to#1707858
Hey Tyler :)

On Wed, Aug 09, 2017 at 03:33:28PM -0500, Tyler Hicks wrote:
> Hey Tycho!
> 
> On 08/09/2017 03:22 PM, Tycho Andersen wrote:
> > On Wed, Aug 09, 2017 at 12:01:53PM -0700, Kees Cook wrote:
> >> This series is the result of Fabricio and I going around a few times
> >> on possible solutions for finding a way to enhance RET_KILL to kill
> >> the process group. There's a lot of ways this could be done, but I
> >> wanted something that felt cleanest. As it happens, Tyler's recent
> >> patch series for logging improvement also needs to know a litte bit
> >> more during filter runs, and the solution for both is to pass back
> >> the matched filter. This lets us examine it here for RET_KILL and
> >> in the future for logging changes.
> >>
> >> The filter passing is patch 1, the new flag for RET_KILL is patch 2.
> >> Some test refactoring is in patch 3 for the RET_DATA ordering, and
> >> patch 4 is the test for the new RET_KILL flag.
> >>
> >> One thing missing is that CRIU will likely need to be updated, since
> >> saving/restoring seccomp filter _rules_ will not include the filter
> >> _flags_ for a process. This can be addressed separately.
> > 
> > Thanks for the heads up, I suppose PTRACE_SECCOMP_GET_FLAGS similar to
> > how PTRACE_SECCOMP_GET_FILTER works will be fine for this. One
> > question is: would we then also need to keep track of the TSYNC flag?
> > I don't think CRIU needs this to be correct, and we can grab the
> > KILL_PROCESS flag from filter->kill_process, so perhaps it's moot.
> 
> Note that the logging changes that I'm working on also introduce a new
> filter flag (as Kees mentioned above). My filter flag is a lot like the
> KILL_PROCESS filter flag in that it is stored as a member of the
> seccomp_filter struct.
> 
> I would think that you'd want to be able to do something like
> PTRACE_SECCOMP_GET_FILTER to (hopefully) future proof CRIU against all
> newly added filter flags.

Yep, the theoretical GET_FLAGS above would handle this, I think. What
I was wondering about is for TSYNC (or any future flags) which aren't
tracked in the struct seccomp_filter; would the existence of GET_FLAGS
mean we need to remember such flags as well somewhere? Not necessary
for CRIU's correctness right now at least, but...

Cheers,

Tycho

[toc] | [prev] | [next] | [standalone]


#1707944

FromKees Cook <keescook@chromium.org>
Date2017-08-09 23:00 +0200
Message-ID<ucGfa-3YO-43@gated-at.bofh.it>
In reply to#1707858
On Wed, Aug 9, 2017 at 1:33 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> Hey Tycho!
>
> On 08/09/2017 03:22 PM, Tycho Andersen wrote:
>> On Wed, Aug 09, 2017 at 12:01:53PM -0700, Kees Cook wrote:
>>> This series is the result of Fabricio and I going around a few times
>>> on possible solutions for finding a way to enhance RET_KILL to kill
>>> the process group. There's a lot of ways this could be done, but I
>>> wanted something that felt cleanest. As it happens, Tyler's recent
>>> patch series for logging improvement also needs to know a litte bit
>>> more during filter runs, and the solution for both is to pass back
>>> the matched filter. This lets us examine it here for RET_KILL and
>>> in the future for logging changes.
>>>
>>> The filter passing is patch 1, the new flag for RET_KILL is patch 2.
>>> Some test refactoring is in patch 3 for the RET_DATA ordering, and
>>> patch 4 is the test for the new RET_KILL flag.
>>>
>>> One thing missing is that CRIU will likely need to be updated, since
>>> saving/restoring seccomp filter _rules_ will not include the filter
>>> _flags_ for a process. This can be addressed separately.
>>
>> Thanks for the heads up, I suppose PTRACE_SECCOMP_GET_FLAGS similar to
>> how PTRACE_SECCOMP_GET_FILTER works will be fine for this. One
>> question is: would we then also need to keep track of the TSYNC flag?
>> I don't think CRIU needs this to be correct, and we can grab the
>> KILL_PROCESS flag from filter->kill_process, so perhaps it's moot.

It does not need to track TSYNC (since the results of that flag are
represented in the filter tree itself).

> Note that the logging changes that I'm working on also introduce a new
> filter flag (as Kees mentioned above). My filter flag is a lot like the
> KILL_PROCESS filter flag in that it is stored as a member of the
> seccomp_filter struct.
>
> I would think that you'd want to be able to do something like
> PTRACE_SECCOMP_GET_FILTER to (hopefully) future proof CRIU against all
> newly added filter flags.

I didn't see an obvious way to extend the existing
PTRACE_SECCOMP_GET_FILTER to also return flags, so I think either a
new function for just flags or a new versioned function for rules and
flags will be needed.

>> Anyway, happy to do this and the userspace part when this lands.

Okay, great! Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web