Re: [PATCH] seccomp: add ptrace commands for suspend/resume

On Tue, Jun 2, 2015 at 12:02 PM, Pavel Emelyanov [off-list ref] wrote:

quoted

quoted

+int suspend_seccomp(struct task_struct *task)
+{
+    int ret = -EACCES;
+
+    spin_lock_irq(&task->sighand->siglock);
+
+    if (!capable(CAP_SYS_ADMIN))
+            goto out;

I am puzzled ;) Why do we need ->siglock? And even if we need it, why
we can't check CAP_SYS_ADMIN lockless?

And I am not sure I understand why do we need the additional security
check, but I leave this to you and Andy.

If you have the rights to trace this task, then you can do anything
the tracee could do without the filtering.

I think _this_ check is required, otherwise the seccomp-ed task (in
filtered mode) fork-s a child, then this child ptrace-attach to parent
(allowed) then suspend its seccomd. And -- we have unpriviledged process
de-seccomped.

The interaction between ptrace and seccomp is already highly screwy,
and seccomp users can often escape using ptrace.  That being said, I'd
rather not further enshrine it.

It might be worth changing the check to verify that the task trying to
suspect seccomp isn't itself subject to seccomp.  That should get most
of the safety.  We are already kind of screwed if someone tries to run
criu under seccomp due to unresolved nesting issues.

--Andy