[PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[Dave Gerlach]

Hi,
There are several instances when one would want to execute out of on-chip
SRAM, such as PM code on ARM platforms, so once again revisiting this
series to allow that in a generic manner. Seems that having a solution for
allowing SRAM to be mapped as executable will help clean up PM code on several
ARM platforms that are using ARM internal __arm_ioremap_exec API
and also open the door for PM support on new platforms like TI AM335x and
AM437x. This was last sent as RFC here [1] and based on comments from Russell
King and Arnd Bergmann has been rewritten to use memremap API rather than
ioremap API, as executable iomem does not really make sense.

I still see several platforms (at-91, imx6, socfpga) that could make use
of this and use the generic sram driver to allocate the SRAM needed for PM.
This series allows direct allocation of SRAM using the generic SRAM driver
that will be already mapped as executable. Otherwise platform SRAM allocation
code must be used or if the generic sram driver is used as-is the memory
must be remapped as executable after it has been mapped normally by the
SRAM driver.

I had sent omap3 series to convert from using old omap sram platform
mapping code to using the generic sram driver instead here [2] which was
based on previous RFC but can easily be rebased on this updated series.
Finally, forthcoming TI am335x and am437x PM code must make use of
it as well, as portions of PM code are moving in to drivers.

Changes from the RFC include:

- Rather than introduce ioremap_exec API, use memremap API, as the concept
  of executable iomem does not make sense; any memory that can used to
  run code should not have io side effects like iomem.
- Rather than having a fallback mapping if a platform does not support
  exec mappings under the memremap API, have the mapping fail, as if you
  are mapping memory as exec, presumably you will then try to run code
  from it which will fail anyway, so it makes more sense to prevent the
  mapping in the first place.
- Update sram driver to use memremap rather than ioremap for exec flags.

Regards,
Dave

[1] http://www.spinics.net/lists/arm-kernel/msg503071.html
[2] http://www.spinics.net/lists/linux-omap/msg128753.html

Dave Gerlach (3):
  ARM: memremap: implement arch_memremap_exec/exec_nocache
  memremap: add MEMREMAP_EXEC and MEMREMAP_EXEC_NOCACHE flags
  misc: SRAM: Add option to map SRAM to allow code execution

 Documentation/devicetree/bindings/sram/sram.txt |  2 ++
 arch/arm/include/asm/io.h                       |  5 ++++
 arch/arm/mm/ioremap.c                           | 16 ++++++++++++
 arch/arm/mm/nommu.c                             | 12 +++++++++
 drivers/misc/sram.c                             |  7 +++++
 include/linux/io.h                              |  2 ++
 kernel/memremap.c                               | 34 ++++++++++++++++++++++++-
 7 files changed, 77 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH 1/3] ARM: memremap: implement arch_memremap_exec/exec_nocache

[Dave Gerlach]

Introduce arch_memremap_exec and arch_memremap_exec_nocache for ARM to
allow mapping of memory as executable. Some platforms have a requirement
to map on-chip memory, such as SRAM, to hold and run code that cannot be
executed in DRAM, such as low-level PM code or code to reconfigure the
DRAM controller itself.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 arch/arm/include/asm/io.h |  5 +++++
 arch/arm/mm/ioremap.c     | 16 ++++++++++++++++
 arch/arm/mm/nommu.c       | 12 ++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 021692c64de3..67476a5add20 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -411,6 +411,11 @@ void __iomem *ioremap_wc(resource_size_t res_cookie, size_t size);
 void iounmap(volatile void __iomem *iomem_cookie);
 #define iounmap iounmap
 
+void *arch_memremap_exec(phys_addr_t phys_addr, size_t size);
+void *arch_memremap_exec_nocache(phys_addr_t phys_addr, size_t size);
+#define arch_memremap_exec arch_memremap_exec
+#define arch_memremap_exec_nocache arch_memremap_exec_nocache
+
 void *arch_memremap_wb(phys_addr_t phys_addr, size_t size);
 #define arch_memremap_wb arch_memremap_wb
 
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index ff0eed23ddf1..5c22a7a0b349 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -419,6 +419,22 @@ __arm_ioremap_exec(phys_addr_t phys_addr, size_t size, bool cached)
 			__builtin_return_address(0));
 }
 
+void *arch_memremap_exec(phys_addr_t phys_addr, size_t size)
+{
+	return (__force void *)arch_ioremap_caller(phys_addr, size,
+						   MT_MEMORY_RWX,
+						   __builtin_return_address(0));
+}
+EXPORT_SYMBOL(arch_memremap_exec);
+
+void *arch_memremap_exec_nocache(phys_addr_t phys_addr, size_t size)
+{
+	return (__force void *)arch_ioremap_caller(phys_addr, size,
+						   MT_MEMORY_RWX_NONCACHED,
+						   __builtin_return_address(0));
+}
+EXPORT_SYMBOL(arch_memremap_exec_nocache);
+
 void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
 {
 	return (__force void *)arch_ioremap_caller(phys_addr, size,
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 2740967727e2..038922133ec4 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -390,6 +390,18 @@ void *arch_memremap_wb(phys_addr_t phys_addr, size_t size)
 	return (void *)phys_addr;
 }
 
+void *arch_memremap_exec(phys_addr_t phys_addr, size_t size)
+{
+	return (void *)phys_addr;
+}
+EXPORT_SYMBOL(arch_memremap_exec);
+
+void *arch_memremap_exec_nocache(phys_addr_t phys_addr, size_t size)
+{
+	return (void *)phys_addr;
+}
+EXPORT_SYMBOL(arch_memremap_exec_nocache);
+
 void __iounmap(volatile void __iomem *addr)
 {
 }
-- 
2.9.3

[PATCH 2/3] memremap: add MEMREMAP_EXEC and MEMREMAP_EXEC_NOCACHE flags

[Dave Gerlach]

Add flags to memremap() for MEMREMAP_EXEC and MEMREMAP_EXEC_NOCACHE
mappings. Mappings provided by these flags will both allow execution,
with MEMREMAP_EXEC_NOCACHE also requiring the memory to be mapped
non-cached. These mappings are useful for architectures that must map
on-chip memory such as SRAM and then copy and execute code from it, such
as code used to reconfigure system memory controllers or the low-level PM
code on ARM platforms.

Also add stubs for both underlying arch_memremap_exec/nocache calls that
return NULL so that if either is attempted from a platform cannot map
memory in such a way will fail.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 include/linux/io.h |  2 ++
 kernel/memremap.c  | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/linux/io.h b/include/linux/io.h
index e2c8419278c1..6668b6b9123b 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -136,6 +136,8 @@ enum {
 	MEMREMAP_WB = 1 << 0,
 	MEMREMAP_WT = 1 << 1,
 	MEMREMAP_WC = 1 << 2,
+	MEMREMAP_EXEC = 1 << 3,
+	MEMREMAP_EXEC_NOCACHE = 1 << 4,
 };
 
 void *memremap(resource_size_t offset, size_t size, unsigned long flags);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index b501e390bb34..47cefc64057d 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -34,6 +34,21 @@ static void *arch_memremap_wb(resource_size_t offset, unsigned long size)
 }
 #endif
 
+#ifndef arch_memremap_exec
+static void *arch_memremap_exec(resource_size_t offset, unsigned long size)
+{
+	return NULL;
+}
+#endif
+
+#ifndef arch_memremap_exec_nocache
+static void *arch_memremap_exec_nocache(resource_size_t offset,
+					unsigned long size)
+{
+	return NULL;
+}
+#endif
+
 static void *try_ram_remap(resource_size_t offset, size_t size)
 {
 	unsigned long pfn = PHYS_PFN(offset);
@@ -48,7 +63,8 @@ static void *try_ram_remap(resource_size_t offset, size_t size)
  * memremap() - remap an iomem_resource as cacheable memory
  * @offset: iomem resource start address
  * @size: size of remap
- * @flags: any of MEMREMAP_WB, MEMREMAP_WT and MEMREMAP_WC
+ * @flags: any of MEMREMAP_WB, MEMREMAP_WT, MEMREMAP_WC, MEMREMAP_EXEC,
+ *	   and MEMREMAP_EXEC_NOCACHE
  *
  * memremap() is "ioremap" for cases where it is known that the resource
  * being mapped does not have i/o side effects and the __iomem
@@ -70,6 +86,16 @@ static void *try_ram_remap(resource_size_t offset, size_t size)
  * MEMREMAP_WC - establish a writecombine mapping, whereby writes may
  * be coalesced together (e.g. in the CPU's write buffers), but is otherwise
  * uncached. Attempts to map System RAM with this mapping type will fail.
+ *
+ * MEMREMAP_EXEC - map memory as cached, as MEMREMAP_WB does, but also
+ * executable to allow for executing code from something like an on-chip
+ * memory. If support for executable mapping is not present on platform
+ * then the mapping will fail.
+ *
+ * MEMREMAP_EXEC_NOCACHE - map memory as non-cached and executable to allow
+ * for executing code from something like an on-chip memory. If support for
+ * executable, non-cached mapping is not present on platform then the
+ * mapping will fail.
  */
 void *memremap(resource_size_t offset, size_t size, unsigned long flags)
 {
@@ -118,6 +144,12 @@ void *memremap(resource_size_t offset, size_t size, unsigned long flags)
 	if (!addr && (flags & MEMREMAP_WC))
 		addr = ioremap_wc(offset, size);
 
+	if (!addr && (flags & MEMREMAP_EXEC))
+		addr = arch_memremap_exec(offset, size);
+
+	if (!addr && (flags & MEMREMAP_EXEC_NOCACHE))
+		addr = arch_memremap_exec_nocache(offset, size);
+
 	return addr;
 }
 EXPORT_SYMBOL(memremap);
-- 
2.9.3

[PATCH 3/3] misc: SRAM: Add option to map SRAM to allow code execution

[Dave Gerlach]

Allow option for mapping SRAM as executable. DT node can specify
"memory-exec" and "memory-exec-nocache" to also map it as non-cached.
This is useful for platforms using the sram driver that need to run
PM code from sram like several ARM platforms.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 Documentation/devicetree/bindings/sram/sram.txt | 2 ++
 drivers/misc/sram.c                             | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/sram/sram.txt b/Documentation/devicetree/bindings/sram/sram.txt
index add48f09015e..8eac557204e5 100644
--- a/Documentation/devicetree/bindings/sram/sram.txt
+++ b/Documentation/devicetree/bindings/sram/sram.txt
@@ -29,6 +29,8 @@ Optional properties in the sram node:
 
 - no-memory-wc : the flag indicating, that SRAM memory region has not to
                  be remapped as write combining. WC is used by default.
+- memory-exec : map range to allow code execution
+- memory-exec-nocache : map range to allow code execution and also non-cached
 
 Required properties in the area nodes:
 
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index f84b53d6ce50..a9ba1be115bb 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -362,6 +362,13 @@ static int sram_probe(struct platform_device *pdev)
 
 	if (of_property_read_bool(pdev->dev.of_node, "no-memory-wc"))
 		sram->virt_base = devm_ioremap(sram->dev, res->start, size);
+	else if (of_property_read_bool(pdev->dev.of_node, "memory-exec"))
+		sram->virt_base = devm_memremap(sram->dev, res->start, size,
+						MEMREMAP_EXEC);
+	else if (of_property_read_bool(pdev->dev.of_node,
+				       "memory-exec-nocache"))
+		sram->virt_base = devm_memremap(sram->dev, res->start, size,
+						MEMREMAP_EXEC_NOCACHE);
 	else
 		sram->virt_base = devm_ioremap_wc(sram->dev, res->start, size);
 	if (!sram->virt_base)
-- 
2.9.3

Re: [PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[Alexandre Belloni]

Hi,

On 27/10/2016 at 13:56:09 -0500, Dave Gerlach wrote :
> There are several instances when one would want to execute out of on-chip
> SRAM, such as PM code on ARM platforms, so once again revisiting this
> series to allow that in a generic manner. Seems that having a solution for
> allowing SRAM to be mapped as executable will help clean up PM code on several
> ARM platforms that are using ARM internal __arm_ioremap_exec API
> and also open the door for PM support on new platforms like TI AM335x and
> AM437x. This was last sent as RFC here [1] and based on comments from Russell
> King and Arnd Bergmann has been rewritten to use memremap API rather than
> ioremap API, as executable iomem does not really make sense.
> 
> I still see several platforms (at-91, imx6, socfpga) that could make use
> of this and use the generic sram driver to allocate the SRAM needed for PM.
> This series allows direct allocation of SRAM using the generic SRAM driver
> that will be already mapped as executable. Otherwise platform SRAM allocation
> code must be used or if the generic sram driver is used as-is the memory
> must be remapped as executable after it has been mapped normally by the
> SRAM driver.
> 

Rockchip also actually needs that.

> I had sent omap3 series to convert from using old omap sram platform
> mapping code to using the generic sram driver instead here [2] which was
> based on previous RFC but can easily be rebased on this updated series.
> Finally, forthcoming TI am335x and am437x PM code must make use of
> it as well, as portions of PM code are moving in to drivers.
> 
> Changes from the RFC include:
> 
> - Rather than introduce ioremap_exec API, use memremap API, as the concept
>   of executable iomem does not make sense; any memory that can used to
>   run code should not have io side effects like iomem.
> - Rather than having a fallback mapping if a platform does not support
>   exec mappings under the memremap API, have the mapping fail, as if you
>   are mapping memory as exec, presumably you will then try to run code
>   from it which will fail anyway, so it makes more sense to prevent the
>   mapping in the first place.
> - Update sram driver to use memremap rather than ioremap for exec flags.
> 
> Regards,
> Dave
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg503071.html
> [2] http://www.spinics.net/lists/linux-omap/msg128753.html
> 
> Dave Gerlach (3):
>   ARM: memremap: implement arch_memremap_exec/exec_nocache
>   memremap: add MEMREMAP_EXEC and MEMREMAP_EXEC_NOCACHE flags
>   misc: SRAM: Add option to map SRAM to allow code execution
> 
>  Documentation/devicetree/bindings/sram/sram.txt |  2 ++
>  arch/arm/include/asm/io.h                       |  5 ++++
>  arch/arm/mm/ioremap.c                           | 16 ++++++++++++
>  arch/arm/mm/nommu.c                             | 12 +++++++++
>  drivers/misc/sram.c                             |  7 +++++
>  include/linux/io.h                              |  2 ++
>  kernel/memremap.c                               | 34 ++++++++++++++++++++++++-
>  7 files changed, 77 insertions(+), 1 deletion(-)
> 

To the whole series:
Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>



-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[Russell King - ARM Linux]

On Thu, Oct 27, 2016 at 01:56:09PM -0500, Dave Gerlach wrote:
> There are several instances when one would want to execute out of on-chip
> SRAM, such as PM code on ARM platforms, so once again revisiting this
> series to allow that in a generic manner. Seems that having a solution for
> allowing SRAM to be mapped as executable will help clean up PM code on several
> ARM platforms that are using ARM internal __arm_ioremap_exec API
> and also open the door for PM support on new platforms like TI AM335x and
> AM437x. This was last sent as RFC here [1] and based on comments from Russell
> King and Arnd Bergmann has been rewritten to use memremap API rather than
> ioremap API, as executable iomem does not really make sense.

This is better, as it avoids the issue that I pointed out last time
around, but I'm still left wondering about the approach.

Sure, having executable SRAM mappings sounds nice and easy, but we're
creating WX mappings.  Folk have spent a while improving the security of
the kernel by ensuring that there are no WX mappings, and this series
reintroduces them.  The sad thing is that any WX mapping which appears
at a known address can be exploited.

"A known address" can be something that appears to be random, but ends
up being the same across the same device type... or can be discovered
by some means.  Eg, consider if the WX mapping is dynamically allocated,
but occurs at exactly the same point at boot - and if this happens with
android phones, consider how many of those are out there.  Or if the
address of the WX mapping is available via some hardware register.
Or...

See Kees Cook's slides at last years kernel summit -
	https://outflux.net/slides/2015/ks/security.pdf

So, I think avoiding WX mappings - mappings should be either W or X but
not both simultaneously (see page 19.)

I guess what I'm angling at is that we don't want memremap_exec(), but
we need an API which changes the permissions of a SRAM mapping between
allowing writes and allowing execution.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[Tony Lindgren]

* Russell King - ARM Linux <linux@armlinux.org.uk> [161107 04:05]:
> On Thu, Oct 27, 2016 at 01:56:09PM -0500, Dave Gerlach wrote:
> > There are several instances when one would want to execute out of on-chip
> > SRAM, such as PM code on ARM platforms, so once again revisiting this
> > series to allow that in a generic manner. Seems that having a solution for
> > allowing SRAM to be mapped as executable will help clean up PM code on several
> > ARM platforms that are using ARM internal __arm_ioremap_exec API
> > and also open the door for PM support on new platforms like TI AM335x and
> > AM437x. This was last sent as RFC here [1] and based on comments from Russell
> > King and Arnd Bergmann has been rewritten to use memremap API rather than
> > ioremap API, as executable iomem does not really make sense.
> 
> This is better, as it avoids the issue that I pointed out last time
> around, but I'm still left wondering about the approach.
> 
> Sure, having executable SRAM mappings sounds nice and easy, but we're
> creating WX mappings.  Folk have spent a while improving the security of
> the kernel by ensuring that there are no WX mappings, and this series
> reintroduces them.  The sad thing is that any WX mapping which appears
> at a known address can be exploited.
> 
> "A known address" can be something that appears to be random, but ends
> up being the same across the same device type... or can be discovered
> by some means.  Eg, consider if the WX mapping is dynamically allocated,
> but occurs at exactly the same point at boot - and if this happens with
> android phones, consider how many of those are out there.  Or if the
> address of the WX mapping is available via some hardware register.
> Or...
> 
> See Kees Cook's slides at last years kernel summit -
> 	https://outflux.net/slides/2015/ks/security.pdf
> 
> So, I think avoiding WX mappings - mappings should be either W or X but
> not both simultaneously (see page 19.)
> 
> I guess what I'm angling at is that we don't want memremap_exec(), but
> we need an API which changes the permissions of a SRAM mapping between
> allowing writes and allowing execution.

That should work just fine. So first copy the code to SRAM,
then set it read-only and exectuable. Note that we need to
restore the state of SRAM every time when returning from
off mode during idle on some SoCs.

Regards,

Tony

Re: [PATCH 0/3] Add memremap executable mapping and extend drivers/misc/sram.c

[Dave Gerlach]

On 11/07/2016 11:43 AM, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@armlinux.org.uk> [161107 04:05]:
>> On Thu, Oct 27, 2016 at 01:56:09PM -0500, Dave Gerlach wrote:
>>> There are several instances when one would want to execute out of on-chip
>>> SRAM, such as PM code on ARM platforms, so once again revisiting this
>>> series to allow that in a generic manner. Seems that having a solution for
>>> allowing SRAM to be mapped as executable will help clean up PM code on several
>>> ARM platforms that are using ARM internal __arm_ioremap_exec API
>>> and also open the door for PM support on new platforms like TI AM335x and
>>> AM437x. This was last sent as RFC here [1] and based on comments from Russell
>>> King and Arnd Bergmann has been rewritten to use memremap API rather than
>>> ioremap API, as executable iomem does not really make sense.
>>
>> This is better, as it avoids the issue that I pointed out last time
>> around, but I'm still left wondering about the approach.
>>
>> Sure, having executable SRAM mappings sounds nice and easy, but we're
>> creating WX mappings.  Folk have spent a while improving the security of
>> the kernel by ensuring that there are no WX mappings, and this series
>> reintroduces them.  The sad thing is that any WX mapping which appears
>> at a known address can be exploited.
>>
>> "A known address" can be something that appears to be random, but ends
>> up being the same across the same device type... or can be discovered
>> by some means.  Eg, consider if the WX mapping is dynamically allocated,
>> but occurs at exactly the same point at boot - and if this happens with
>> android phones, consider how many of those are out there.  Or if the
>> address of the WX mapping is available via some hardware register.
>> Or...
>>
>> See Kees Cook's slides at last years kernel summit -
>> 	https://outflux.net/slides/2015/ks/security.pdf
>>
>> So, I think avoiding WX mappings - mappings should be either W or X but
>> not both simultaneously (see page 19.)
>>
>> I guess what I'm angling at is that we don't want memremap_exec(), but
>> we need an API which changes the permissions of a SRAM mapping between
>> allowing writes and allowing execution.
>
> That should work just fine. So first copy the code to SRAM,
> then set it read-only and exectuable. Note that we need to
> restore the state of SRAM every time when returning from
> off mode during idle on some SoCs.

Thanks for all the comments. This seems like a reasonable concern. I do 
agree that we would need to be able to briefly restore write permission 
for things like Tony has described.

In fact, I suppose we would need this ability for every copy so that  we 
switch from exec to write and do the copy, then switch back to read-only 
executable. We have situations where we copy multiple things, from 
drivers that don't necessarily have knowledge of one another to the SRAM 
space at different times.

Any opinions on where this API should live?

Regards,
Dave

>
> Regards,
>
> Tony
>