Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933349AbeAKKZV (ORCPT + 1 other); Thu, 11 Jan 2018 05:25:21 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:37112 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753778AbeAKKZT (ORCPT ); Thu, 11 Jan 2018 05:25:19 -0500 Date: Thu, 11 Jan 2018 10:24:00 +0000 From: Russell King - ARM Linux To: Kees Cook Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Christian Borntraeger , "Peter Zijlstra (Intel)" , linux-arm-kernel@lists.infradead.org, Linus Torvalds , David Windsor , Alexander Viro , Andrew Morton , Andy Lutomirski , Christoph Hellwig , Christoph Lameter , "David S. Miller" , Laura Abbott , Mark Rutland , "Martin K. Petersen" , Paolo Bonzini , Christoffer Dall , Dave Kleikamp , Jan Kara , Luis de Bethencourt , Marc Zyngier , Rik van Riel , Matthew Garrett , linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, netdev@vger.kernel.org, linux-mm@kvack.org, kernel-hardening@lists.openwall.com Subject: Re: [PATCH 34/38] arm: Implement thread_struct whitelist for hardened usercopy Message-ID: <20180111102400.GT17719@n2100.armlinux.org.uk> References: <1515636190-24061-1-git-send-email-keescook@chromium.org> <1515636190-24061-35-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1515636190-24061-35-git-send-email-keescook@chromium.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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 > Cc: Ingo Molnar > Cc: Christian Borntraeger > Cc: "Peter Zijlstra (Intel)" > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Kees Cook > --- > 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