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

Russell King - ARM Linux
2018-01-11T10:24:00Z

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@armlinux.org.uk>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Kees Cook <keescook@chromium.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