linux-kernel - Re: [PATCH v2] seccomp: switch to using asm-generic for seccomp.h

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] [day] [month] [year] [list]
Date:	Tue, 3 Mar 2015 09:56:02 -0800
From:	Kees Cook <keescook@...omium.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	AKASHI Takahiro <takahiro.akashi@...aro.org>,
	Russell King <linux@....linux.org.uk>,
	Michal Simek <monstr@...str.eu>,
	Ralf Baechle <ralf@...ux-mips.org>,
	"James E.J. Bottomley" <jejb@...isc-linux.org>,
	Helge Deller <deller@....de>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	linux390@...ibm.com, "David S. Miller" <davem@...emloft.net>,
	"x86@...nel.org" <x86@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Laura Abbott <lauraa@...eaurora.org>,
	Will Deacon <will.deacon@....com>,
	Daniel Borkmann <dborkman@...hat.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	James Hogan <james.hogan@...tec.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Linux MIPS Mailing List <linux-mips@...ux-mips.org>,
	linux-parisc <linux-parisc@...r.kernel.org>,
	linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
	linux-sh <linux-sh@...r.kernel.org>,
	sparclinux <sparclinux@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] seccomp: switch to using asm-generic for seccomp.h

On Tue, Mar 3, 2015 at 12:30 AM, Ingo Molnar <mingo@...nel.org> wrote:
>
> * Kees Cook <keescook@...omium.org> wrote:
>
>> Most architectures don't need to do anything special for the strict
>> seccomp syscall entries. Remove the redundant headers and reduce the
>> others.
>
>>  19 files changed, 27 insertions(+), 137 deletions(-)
>
> Lovely cleanup factor.
>
> Just to make sure, are you sure the 32-bit details are identical
> across architectures?

I did "gcc -E -dM" style output comparisons on the architectures I had
compilers for, and the buildbot hasn't complained on any of the others
(though see the bottom of this email).

>
> For example some architectures did this:
>
>> --- a/arch/microblaze/include/asm/seccomp.h
>> +++ /dev/null
>> @@ -1,16 +0,0 @@
>> -#ifndef _ASM_MICROBLAZE_SECCOMP_H
>> -#define _ASM_MICROBLAZE_SECCOMP_H
>> -
>> -#include <linux/unistd.h>
>> -
>> -#define __NR_seccomp_read            __NR_read
>> -#define __NR_seccomp_write           __NR_write
>> -#define __NR_seccomp_exit            __NR_exit
>> -#define __NR_seccomp_sigreturn               __NR_sigreturn
>> -
>> -#define __NR_seccomp_read_32         __NR_read
>> -#define __NR_seccomp_write_32                __NR_write
>> -#define __NR_seccomp_exit_32         __NR_exit
>> -#define __NR_seccomp_sigreturn_32    __NR_sigreturn

The asm-generic uses the same syscall numbers from both 64 and 32,
which matches most architectures, and those are the ones that had
their seccomp.h entirely eliminated.

> others did this:
>
>> diff --git a/arch/x86/include/asm/seccomp_64.h b/arch/x86/include/asm/seccomp_64.h
>> deleted file mode 100644
>> index 84ec1bd161a5..000000000000
>> --- a/arch/x86/include/asm/seccomp_64.h
>> +++ /dev/null
>> @@ -1,17 +0,0 @@
>> -#ifndef _ASM_X86_SECCOMP_64_H
>> -#define _ASM_X86_SECCOMP_64_H
>> -
>> -#include <linux/unistd.h>
>> -#include <asm/ia32_unistd.h>
>> -
>> -#define __NR_seccomp_read __NR_read
>> -#define __NR_seccomp_write __NR_write
>> -#define __NR_seccomp_exit __NR_exit
>> -#define __NR_seccomp_sigreturn __NR_rt_sigreturn
>> -
>> -#define __NR_seccomp_read_32 __NR_ia32_read
>> -#define __NR_seccomp_write_32 __NR_ia32_write
>> -#define __NR_seccomp_exit_32 __NR_ia32_exit
>> -#define __NR_seccomp_sigreturn_32 __NR_ia32_sigreturn
>> -
>> -#endif /* _ASM_X86_SECCOMP_64_H */

Well, this was x86's split config that was consolidated into the file below:

>
> While in yet another case you kept the syscall mappings:
>
>> --- a/arch/x86/include/asm/seccomp.h
>> +++ b/arch/x86/include/asm/seccomp.h
>> @@ -1,5 +1,20 @@
>> +#ifndef _ASM_X86_SECCOMP_H
>> +#define _ASM_X86_SECCOMP_H
>> +
>> +#include <asm/unistd.h>
>> +
>> +#ifdef CONFIG_COMPAT
>> +#include <asm/ia32_unistd.h>
>> +#define __NR_seccomp_read_32         __NR_ia32_read
>> +#define __NR_seccomp_write_32                __NR_ia32_write
>> +#define __NR_seccomp_exit_32         __NR_ia32_exit
>> +#define __NR_seccomp_sigreturn_32    __NR_ia32_sigreturn
>> +#endif
>> +
>>  #ifdef CONFIG_X86_32
>> -# include <asm/seccomp_32.h>
>> -#else
>> -# include <asm/seccomp_64.h>
>> +#define __NR_seccomp_sigreturn               __NR_sigreturn
>>  #endif
>> +
>> +#include <asm-generic/seccomp.h>
>> +
>> +#endif /* _ASM_X86_SECCOMP_H */
>
> It might all be correct, but it's not obvious to me.

The x86 change was the most complex as it removed a seccomp_32. and
seccomp_64.h file and merged into a single asm/seccomp.h to provide
overrides for the _32 #defines.

However, in looking at it now... I see some flip/flopping of
__NR_sigreturn and __NR_rt_sigreturn between some of the
architectures. Let me study that and send a v3. I think there are some
accidental changes on microblaze and powerpc.

-Kees

-- 
Kees Cook
Chrome OS Security
--
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/