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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrVYHYfogj3nTY-3ui87+tVi3mG3D4=Xdk-_MpisG8BczA@mail.gmail.com>
Date:	Tue, 2 Jun 2015 12:27:42 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Pavel Emelyanov <xemul@...allels.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Tycho Andersen <tycho.andersen@...onical.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linux API <linux-api@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>,
	Will Drewry <wad@...omium.org>,
	Roland McGrath <roland@...k.frob.com>,
	"Serge E. Hallyn" <serge.hallyn@...ntu.com>
Subject: Re: [PATCH] seccomp: add ptrace commands for suspend/resume

On Tue, Jun 2, 2015 at 12:02 PM, Pavel Emelyanov <xemul@...allels.com> wrote:
>
>>> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/