linux-kernel - Re: [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET

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]

Date:	Thu, 24 May 2012 13:24:37 -0500
From:	Will Drewry <wad@...omium.org>
To:	Indan Zupancic <indan@....nu>
Cc:	linux-kernel@...r.kernel.org, mcgrathr@...gle.com, hpa@...or.com,
	netdev@...isplace.org, linux-security-module@...r.kernel.org,
	kernel-hardening@...ts.openwall.com, mingo@...hat.com,
	oleg@...hat.com, peterz@...radead.org, rdunlap@...otime.net,
	tglx@...utronix.de, luto@....edu, serge.hallyn@...onical.com,
	pmoore@...hat.com, akpm@...ux-foundation.org, corbet@....net,
	markus@...omium.org, coreyb@...ux.vnet.ibm.com,
	keescook@...omium.org, viro@...iv.linux.org.uk, jmorris@...ei.org
Subject: Re: [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET_TRACE

On Thu, May 24, 2012 at 12:54 PM, Indan Zupancic <indan@....nu> wrote:
> On Thu, May 24, 2012 18:07, Will Drewry wrote:
>> Ensure that consumers of the PTRACE_EVENT_SECCOMP notification
>> cannot change the system call number for the traced task
>> without it resulting in the system call being skipped.
>>
>> Traditionally, tracers will set the system call number to
>> -1 to skip the system call. This behavior will work as expected
>> but the tracer will be unable to remap the system call to a valid
>> system call after the seccomp policy has been evaluated.
>>
>> Signed-off-by: Will Drewry <wad@...omium.org>
>> ---
>>  kernel/seccomp.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index ee376be..33f0ad6 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -425,6 +425,10 @@ int __secure_computing(int this_syscall)
>>                        */
>>                       if (fatal_signal_pending(current))
>>                               break;
>> +                     /* Skip the system call if the tracer changed it. */
>> +                     if (this_syscall !=
>> +                         syscall_get_nr(current, task_pt_regs(current)))
>> +                             goto skip;
>>                       return 0;
>>               case SECCOMP_RET_ALLOW:
>>                       return 0;
>> --
>
> This patch doesn't make any sense whatsoever. You can't know why a system
> call was blocked by a seccomp filter, assuming it's always because of the
> system call number is wrong.

All this does is assert that the tracer can't change the syscall
number without it skipping the call.  If seccomp returned
SECCOMP_RET_TRACE because the argument to open was O_RDWR, then
everything is fine.

> Also, you don't check if an allowed system call is changed into a denied
> one, so this doesn't protect against ptracers bypassing seccomp filters.

This enforces that the system call that is going to be executed is the
one that triggered SECCOMP_RET_TRACE.  That means seccomp is
delegating the go/no-go decision to the tracer.  I don't understand
your assertion here.  This code doesn't affect the PTRACE_SYSCALL
case.

> And one of the main points of PTRACE_EVENT_SECCOMP events was that it's
> useful for cases that can't be handled or decided by the seccomp filter.
> Then taking away the ability to change the syscall number makes it a lot
> less useful.

Do you have a valid case where you want to remap one system call to
another without the ability to also handle the syscall exit path and
do any fixups?  I've mostly just seen skip, allow, update arguments -
not swapping the entire syscall.  That said, it's possible.  you could
do all sorts of weird things with ptrace if you want :)

> Either do the seccomp test before or after ptrace, or both, but please
> don't introduce ad hoc checks like this.

I don't feel strongly about this RFC, but I don't believe that
expectations are being changed dramatically.

thanks!
will
--
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/