Re: [PATCH v3 4/4] arm64: compat: Add KUSER_HELPERS config option

From: Catalin Marinas <catalin.marinas@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 4/4] arm64: compat: Add KUSER_HELPERS config option
Date: Thu, 4 Apr 2019 15:51:23 +0100	[thread overview]
Message-ID: <20190404145123.GL43584@arrakis.emea.arm.com> (raw)
In-Reply-To: <d9f20416-1482-1f94-e57c-b2a74c4e926b@arm.com>

On Thu, Apr 04, 2019 at 03:07:43PM +0100, Vincenzo Frascino wrote:
> On 04/04/2019 11:08, Catalin Marinas wrote:
> > On Tue, Apr 02, 2019 at 05:27:57PM +0100, Vincenzo Frascino wrote:
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 7e34b9eba5de..aa28884a2376 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -1494,6 +1494,34 @@ config COMPAT
> >>  
> >>  	  If you want to execute 32-bit userspace applications, say Y.
> >>  
> >> +config KUSER_HELPERS
> >> +	bool "Enable kuser helpers page for 32 bit applications."
> >> +	depends on COMPAT
> >> +	default y
> >> +	help
> >> +	  Warning: disabling this option may break 32-bit user programs.
> >> +
> >> +	  Provide kuser helpers to compat tasks. The kernel provides
> >> +	  helper code to userspace in read only form at a fixed location
> >> +	  to allow userspace to be independent of the CPU type fitted to
> >> +	  the system. This permits binaries to be run on ARMv4 through
> >> +	  to ARMv8 without modification.
> >> +
> >> +	  See Documentation/arm/kernel_user_helpers.txt for details.
> >> +
> >> +	  However, the fixed address nature of these helpers can be used
> >> +	  by ROP (return orientated programming) authors when creating
> >> +	  exploits.
> >> +
> >> +	  If all of the binaries and libraries which run on your platform
> >> +	  are built specifically for your platform, and make no use of
> >> +	  these helpers, then you can turn this option off to hinder
> >> +	  such exploits. However, in that case, if a binary or library
> >> +	  relying on those helpers is run, it will not function correctly.
> >> +
> >> +	  Say N here only if you are absolutely certain that you do not
> >> +	  need these helpers; otherwise, the safe option is to say Y.
> > 
> > In order to close the potential issue with the user unmapping the
> > vectors page in the 64K configuration, we'd need the change below as
> > well (on top of the TASK_SIZE_32 patch I queued for -rc4).
> > 
> > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> > index 228af56ece48..fcd0e691b1ea 100644
> > --- a/arch/arm64/include/asm/processor.h
> > +++ b/arch/arm64/include/asm/processor.h
> > @@ -57,7 +57,7 @@
> >  #define TASK_SIZE_64		(UL(1) << vabits_user)
> >  
> >  #ifdef CONFIG_COMPAT
> > -#ifdef CONFIG_ARM64_64K_PAGES
> > +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> >  /*
> >   * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> >   * by the compat vectors page.
> > 
> 
> I agree, this change is required when the new config option is added. Do you
> want me to send an additional patch or will this be folded with the existing series?

Folding it into patch 4/4 would be best but our arm64 for-next/core is
based on -rc3 and the TASK_SIZE_32 reduction didn't go in yet:

https://lore.kernel.org/linux-arm-kernel/20190401113014.20866-1-vincenzo.frascino@arm.com/

I can drop the TASK_SIZE_32 patch from for-next/fixes (arguably, it's
not a regression and we cc stable anyway) and Will can merge it before
this series. Otherwise, the fixup above can go in at 5.2-rc1.

Will, any preference?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel