netdev - Re: [PATCH 34/38] arm: Implement thread_struct whitelist for hardened usercopy

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: <20180111102400.GT17719@n2100.armlinux.org.uk>
Date:   Thu, 11 Jan 2018 10:24:00 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Kees Cook <keescook@...omium.org>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        linux-arm-kernel@...ts.infradead.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        David Windsor <dave@...lcore.net>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Christoph Hellwig <hch@...radead.org>,
        Christoph Lameter <cl@...ux.com>,
        "David S. Miller" <davem@...emloft.net>,
        Laura Abbott <labbott@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Dave Kleikamp <dave.kleikamp@...cle.com>,
        Jan Kara <jack@...e.cz>,
        Luis de Bethencourt <luisbg@...nel.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Rik van Riel <riel@...hat.com>,
        Matthew Garrett <mjg59@...gle.com>,
        linux-fsdevel@...r.kernel.org, linux-arch@...r.kernel.org,
        netdev@...r.kernel.org, linux-mm@...ck.org,
        kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 34/38] arm: Implement thread_struct whitelist for
 hardened usercopy

On Wed, Jan 10, 2018 at 06:03:06PM -0800, Kees Cook wrote:
> ARM does not carry FPU state in the thread structure, so it can declare
> no usercopy whitelist at all.

This comment seems to be misleading.  We have stored FP state in the
thread structure for a long time - for example, VFP state is stored
in thread->vfpstate.hard, so we _do_ have floating point state in
the thread structure.

What I think this commit message needs to describe is why we don't
need a whitelist _despite_ having FP state in the thread structure.

At the moment, the commit message is making me think that this patch
is wrong and will introduce a regression.

Thanks.

> 
> Cc: Russell King <linux@...linux.org.uk>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Christian Borntraeger <borntraeger@...ibm.com>
> Cc: "Peter Zijlstra (Intel)" <peterz@...radead.org>
> Cc: linux-arm-kernel@...ts.infradead.org
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
>  arch/arm/Kconfig                 | 1 +
>  arch/arm/include/asm/processor.h | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 51c8df561077..3ea00d65f35d 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -50,6 +50,7 @@ config ARM
>  	select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
>  	select HAVE_ARCH_MMAP_RND_BITS if MMU
>  	select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
> +	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
>  	select HAVE_ARCH_TRACEHOOK
>  	select HAVE_ARM_SMCCC if CPU_V7
>  	select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
> diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
> index 338cbe0a18ef..01a41be58d43 100644
> --- a/arch/arm/include/asm/processor.h
> +++ b/arch/arm/include/asm/processor.h
> @@ -45,6 +45,13 @@ struct thread_struct {
>  	struct debug_info	debug;
>  };
>  
> +/* Nothing needs to be usercopy-whitelisted from thread_struct. */
> +static inline void arch_thread_struct_whitelist(unsigned long *offset,
> +						unsigned long *size)
> +{
> +	*offset = *size = 0;
> +}
> +
>  #define INIT_THREAD  {	}
>  
>  #define start_thread(regs,pc,sp)					\
> -- 
> 2.7.4
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up