Nokia N900: insecure W+X mapping at 0xd0050000

Hi!

Insecure W+X mappings, who cares about those? I have 7 pages...

Pavel

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.16.0-rc3-next-20180302 (pavel at duo) (gcc
version 4.7.2 (GC
C)) #70 Fri Mar 2 10:16:00 CET 2018
[    0.000000] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7),
cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT
nonaliasing instruction cac
...
[    1.266693] ------------[ cut here ]------------
[    1.271606] WARNING: CPU: 0 PID: 1 at lib/refcount.c:187
refcount_sub_and_test+0x94/0xa8
[    1.280181] refcount_t: underflow; use-after-free.
[    1.285247] Modules linked in:
...
[    4.557220] devtmpfs: mounted
[    4.569610] Freeing unused kernel memory: 1024K
[    4.581481] ------------[ cut here ]------------
[    4.592376] WARNING: CPU: 0 PID: 1 at arch/arm/mm/dump.c:253
note_page+0x2f8/0x328
[    4.606658] arm/mm: Found insecure W+X mapping at address
0xd0050000
[    4.619781] Modules linked in:
[    4.629272] CPU: 0 PID: 1 Comm: swapper Tainted: G        W
4.16.0-rc3-next-20180302 #70
[    4.645111] Hardware name: Nokia RX-51 board
[    4.655944] [<c010d6cc>] (unwind_backtrace) from [<c010b560>]
(show_stack+0x10/0x14)
[    4.670593] [<c010b560>] (show_stack) from [<c0127dec>]
(__warn+0xe8/0x110)
[    4.684295] [<c0127dec>] (__warn) from [<c0127edc>]
(warn_slowpath_fmt+0x38/0x48)
[    4.698455] [<c0127edc>] (warn_slowpath_fmt) from [<c0113aa8>]
(note_page+0x2f8/0x328)
[    4.713134] [<c0113aa8>] (note_page) from [<c0113b60>]
(walk_pgd+0x88/0x178)
[    4.726837] [<c0113b60>] (walk_pgd) from [<c0113d28>]
(ptdump_check_wx+0x64/0xb8)
[    4.741210] [<c0113d28>] (ptdump_check_wx) from [<c0716428>]
(kernel_init+0x24/0x108)
[    4.755859] [<c0716428>] (kernel_init) from [<c01010e8>]
(ret_from_fork+0x14/0x2c)
[    4.770294] Exception stack(0xce049fb0 to 0xce049ff8)
[    4.781921] 9fa0:                                     00000000
00000000 00000000 00000000
[    4.796966] 9fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    4.812011] 9fe0: 00000000 00000000 00000000 00000000 00000013
00000000
[    4.825378] ---[ end trace dcb3a72772bbfe7c ]---
[    4.837005] Checked W+X mappings: FAILED, 7 W+X pages found

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180308/ef9e20dd/attachment.sig>

Is this with CONFIG_DEBUG_WX=y?

My guess is that it's for mapping the PM assembly to SRAM. This is already fixed for am335x that is using drivers/misc/sram*.c. I think omap2 - omap4 still need fixing if this is the culprit.

Regards,

Tony

Adding Dave to Cc, here's a quick fix for this one.

Regards,

Tony

8< -------------------

From: Tony Lindgren <[email protected]>
Date: Thu, 8 Mar 2018 08:41:16 -0800
Subject: [PATCH] ARM: OMAP: Fix SRAM W+X mapping

We are still using custom SRAM code for some SoCs and are not marking the PM code mapped to SRAM as read-only and executable after we're done. With CONFIG_DEBUG_WX=y, we will get "Found insecure W+X mapping at address" warning.

Let's fix this issue the same way as commit 728bbe75c82f ("misc: sram:
Introduce support code for protect-exec sram type") is doing for drivers/misc/sram-exec.c.

Note that eventually we should be using sram-exec.c for all SoCs.

---
 arch/arm/plat-omap/sram.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

@@ -23,6 +23,7 @@
 #include <asm/fncpy.h>
 #include <asm/tlb.h>
 #include <asm/cacheflush.h>
+#include <asm/set_memory.h>
 
 #include <asm/mach/map.h>
 

@@ -96,3 +97,21 @@ void __init omap_map_sram(unsigned long start, unsigned long size,
 	memset_io(omap_sram_base + omap_sram_skip, 0,
 		  omap_sram_size - omap_sram_skip);
 }
+
+static int __init omap_sram_lock(void)
+{
+	unsigned long base;
+	int pages;
+
+	if (!omap_sram_base || !omap_sram_size)
+		return 0;
+
+	base = (unsigned long)omap_sram_base;
+	pages = PAGE_ALIGN(omap_sram_size) / PAGE_SIZE;
+
+	set_memory_ro((unsigned long)base, pages);
+	set_memory_x((unsigned long)base, pages);
+
+	return 0;
+}
+late_initcall(omap_sram_lock);

2.16.2

Hi!

I did a quick testing and warning is gone. Thanks!

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180308/10049589/attachment.sig>

Turns out init time configuration is not enough here. It causes an oops on omap3 after off mode coming back from idle. So we need to reconfigure things when omap_sram_push() is called after idle too.

Below is an updated patch.

Regards,

Tony

8< ----------------------

From: Tony Lindgren <[email protected]>
Date: Wed, 21 Mar 2018 08:16:29 -0700
Subject: [PATCH] ARM: OMAP: Fix SRAM W+X mapping

We are still using custom SRAM code for some SoCs and are not marking the PM code mapped to SRAM as read-only and executable after we're done. With CONFIG_DEBUG_WX=y, we will get "Found insecure W+X mapping at address" warning.

Let's fix this issue the same way as commit 728bbe75c82f ("misc: sram:
Introduce support code for protect-exec sram type") is doing for drivers/misc/sram-exec.c.

On omap3, we need to restore SRAM when returning from off mode after idle, so init time configuration is not enough.

And as we no longer have users for omap_sram_push_address() we can make it static while at it.

Note that eventually we should be using sram-exec.c for all SoCs.

---
 arch/arm/plat-omap/include/plat/sram.h | 11 +----------
 arch/arm/plat-omap/sram.c              | 36 +++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 11 deletions(-)

@@ -5,13 +5,4 @@ void omap_map_sram(unsigned long start, unsigned long size,
 			unsigned long skip, int cached);
 void omap_sram_reset(void);
 
-extern void *omap_sram_push_address(unsigned long size);
-
-/* Macro to push a function to the internal SRAM, using the fncpy API */
-#define omap_sram_push(funcp, size) ({				\
-	typeof(&(funcp)) _res = NULL;				\
-	void *_sram_address = omap_sram_push_address(size);	\
-	if (_sram_address)					\
-		_res = fncpy(_sram_address, &(funcp), size);	\
-	_res;							\
-})
+extern void *omap_sram_push(void *funcp, unsigned long size);

@@ -23,6 +23,7 @@
 #include <asm/fncpy.h>
 #include <asm/tlb.h>
 #include <asm/cacheflush.h>
+#include <asm/set_memory.h>
 
 #include <asm/mach/map.h>
 

@@ -42,7 +43,7 @@ static void __iomem *omap_sram_ceil;
  * Note that fncpy requires the returned address to be aligned
  * to an 8-byte boundary.
  */
-void *omap_sram_push_address(unsigned long size)
+static void *omap_sram_push_address(unsigned long size)
 {
 	unsigned long available, new_ceil = (unsigned long)omap_sram_ceil;
 

@@ -60,6 +61,30 @@ void *omap_sram_push_address(unsigned long size)
 	return (void *)omap_sram_ceil;
 }
 
+void *omap_sram_push(void *funcp, unsigned long size)
+{
+	void *sram;
+	unsigned long base;
+	int pages;
+	void *dst = NULL;
+
+	sram = omap_sram_push_address(size);
+	if (!sram)
+		return NULL;
+
+	base = (unsigned long)sram & PAGE_MASK;
+	pages = PAGE_ALIGN(size) / PAGE_SIZE;
+
+	set_memory_rw(base, pages);
+
+	dst = fncpy(sram, funcp, size);
+
+	set_memory_ro(base, pages);
+	set_memory_x(base, pages);
+
+	return dst;
+}
+
 /*
  * The SRAM context is lost during off-idle and stack
  * needs to be reset.

@@ -75,6 +100,9 @@ void omap_sram_reset(void)
 void __init omap_map_sram(unsigned long start, unsigned long size,
 				 unsigned long skip, int cached)
 {
+	unsigned long base;
+	int pages;
+
 	if (size == 0)
 		return;
 

@@ -95,4 +123,10 @@ void __init omap_map_sram(unsigned long start, unsigned long size,
 	 */
 	memset_io(omap_sram_base + omap_sram_skip, 0,
 		  omap_sram_size - omap_sram_skip);
+
+	base = (unsigned long)omap_sram_base;
+	pages = PAGE_ALIGN(omap_sram_size) / PAGE_SIZE;
+
+	set_memory_ro(base, pages);
+	set_memory_x(base, pages);
 }

2.16.2