Skip to contentSkip to conversation

arm64: mm: Mark .rodata as RO

PATCH Superseded Jeremy Linton 4 participants 5 messages Feb 19, 2016 – Feb 26, 2016 arm64

lore
Share & tools
lore

Jeremy Linton

author
#0 lore raw

PATCH v3 1/1 arm64: mm: Mark .rodata as RO

Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. It also adds the .rodata section to the mem_init banner.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S |  5 +++--
 arch/arm64/mm/init.c            |  4 +++-
 arch/arm64/mm/mmu.c             | 17 +++++++++++++----
 3 files changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +3 -2
index 8f4fc2c..9208f53 100644
@@ -114,8 +114,9 @@ SECTIONS
 		*(.got)			/* Global offset table		*/
 	}
 
-	RO_DATA(PAGE_SIZE)
-	EXCEPTION_TABLE(8)
+	ALIGN_DEBUG_RO_MIN(0)
+	RO_DATA(PAGE_SIZE)		/* everything from this point to */
+	EXCEPTION_TABLE(8)		/* _etext will be marked RO NX   */
 	NOTES
 
 	ALIGN_DEBUG_RO_MIN(PAGE_SIZE)
arch/arm64/mm/init.c +3 -1
index 5dd0831..41be7db 100644
@@ -325,6 +325,7 @@ void __init mem_init(void)
 		  "    memory  : 0x%16lx - 0x%16lx   (%6ld MB)\n"
 		  "      .init : 0x%p" " - 0x%p" "   (%6ld KB)\n"
 		  "      .text : 0x%p" " - 0x%p" "   (%6ld KB)\n"
+		  "    .rodata : 0x%p" " - 0x%p" "   (%6ld KB)\n"
 		  "      .data : 0x%p" " - 0x%p" "   (%6ld KB)\n",
 #ifdef CONFIG_KASAN
 		  MLG(KASAN_SHADOW_START, KASAN_SHADOW_END),

@@ -341,7 +342,8 @@ void __init mem_init(void)
 		  MLM(MODULES_VADDR, MODULES_END),
 		  MLM(PAGE_OFFSET, (unsigned long)high_memory),
 		  MLK_ROUNDUP(__init_begin, __init_end),
-		  MLK_ROUNDUP(_text, _etext),
+		  MLK_ROUNDUP(_text, __start_rodata),
+		  MLK_ROUNDUP(__start_rodata, _etext),
 		  MLK_ROUNDUP(_sdata, _edata));
 
 #undef MLK
arch/arm64/mm/mmu.c +13 -4
index ab69a99..a18dfd0 100644
@@ -453,10 +453,18 @@ static void __init map_mem(pgd_t *pgd)
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void)
 {
-	create_mapping_late(__pa(_stext), (unsigned long)_stext,
-				(unsigned long)_etext - (unsigned long)_stext,
-				PAGE_KERNEL_ROX);
+	unsigned long section_size;
 
+	section_size = (unsigned long)__start_rodata - (unsigned long)_stext;
+	create_mapping_late(__pa(_stext), (unsigned long)_stext,
+			    section_size, PAGE_KERNEL_ROX);
+	/*
+	 * mark .rodata as read only. Use _etext rather than __end_rodata to
+	 * cover NOTES and EXCEPTION_TABLE.
+	 */
+	section_size = (unsigned long)_etext - (unsigned long)__start_rodata;
+	create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
+			    section_size, PAGE_KERNEL_RO);
 }
 #endif
 

@@ -486,7 +494,8 @@ static void __init map_kernel_chunk(pgd_t *pgd, void *va_start, void *va_end,
 static void __init map_kernel(pgd_t *pgd)
 {
 
-	map_kernel_chunk(pgd, _stext, _etext, PAGE_KERNEL_EXEC);
+	map_kernel_chunk(pgd, _stext, __start_rodata, PAGE_KERNEL_EXEC);
+	map_kernel_chunk(pgd, __start_rodata, _etext, PAGE_KERNEL);
 	map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC);
 	map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);
Signature · 1 line
2.4.3
#1 lore raw

PATCH v3 1/1 arm64: mm: Mark .rodata as RO

Quoted 5 lines · Jeremy Linton

Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. It also adds the .rodata section to the mem_init banner.

Signed-off-by Jeremy Linton <[email protected]>
Reviewed-by Kees Cook <[email protected]>

-Kees

Quoted 83 lines
---
 arch/arm64/kernel/vmlinux.lds.S |  5 +++--
 arch/arm64/mm/init.c            |  4 +++-
 arch/arm64/mm/mmu.c             | 17 +++++++++++++----
 3 files changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +3 -2
index 8f4fc2c..9208f53 100644
@@ -114,8 +114,9 @@ SECTIONS
                *(.got)                 /* Global offset table          */
        }
 
-       RO_DATA(PAGE_SIZE)
-       EXCEPTION_TABLE(8)
+       ALIGN_DEBUG_RO_MIN(0)
+       RO_DATA(PAGE_SIZE)              /* everything from this point to */
+       EXCEPTION_TABLE(8)              /* _etext will be marked RO NX   */
        NOTES
 
        ALIGN_DEBUG_RO_MIN(PAGE_SIZE)
arch/arm64/mm/init.c +3 -1
index 5dd0831..41be7db 100644
@@ -325,6 +325,7 @@ void __init mem_init(void)
                  "    memory  : 0x%16lx - 0x%16lx   (%6ld MB)\n"
                  "      .init : 0x%p" " - 0x%p" "   (%6ld KB)\n"
                  "      .text : 0x%p" " - 0x%p" "   (%6ld KB)\n"
+                 "    .rodata : 0x%p" " - 0x%p" "   (%6ld KB)\n"
                  "      .data : 0x%p" " - 0x%p" "   (%6ld KB)\n",
 #ifdef CONFIG_KASAN
                  MLG(KASAN_SHADOW_START, KASAN_SHADOW_END),

@@ -341,7 +342,8 @@ void __init mem_init(void)
                  MLM(MODULES_VADDR, MODULES_END),
                  MLM(PAGE_OFFSET, (unsigned long)high_memory),
                  MLK_ROUNDUP(__init_begin, __init_end),
-                 MLK_ROUNDUP(_text, _etext),
+                 MLK_ROUNDUP(_text, __start_rodata),
+                 MLK_ROUNDUP(__start_rodata, _etext),
                  MLK_ROUNDUP(_sdata, _edata));
 
 #undef MLK
arch/arm64/mm/mmu.c +13 -5
index ab69a99..a18dfd0 100644
@@ -453,10 +453,18 @@ static void __init map_mem(pgd_t *pgd)
 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void)
 {
-       create_mapping_late(__pa(_stext), (unsigned long)_stext,
-                               (unsigned long)_etext - (unsigned long)_stext,
-                               PAGE_KERNEL_ROX);
+       unsigned long section_size;
 
+       section_size = (unsigned long)__start_rodata - (unsigned long)_stext;
+       create_mapping_late(__pa(_stext), (unsigned long)_stext,
+                           section_size, PAGE_KERNEL_ROX);
+       /*
+        * mark .rodata as read only. Use _etext rather than __end_rodata to
+        * cover NOTES and EXCEPTION_TABLE.
+        */
+       section_size = (unsigned long)_etext - (unsigned long)__start_rodata;
+       create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
+                           section_size, PAGE_KERNEL_RO);
 }
 #endif
 

@@ -486,7 +494,8 @@ static void __init map_kernel_chunk(pgd_t *pgd, void *va_start, void *va_end,
 static void __init map_kernel(pgd_t *pgd)
 {
 
-       map_kernel_chunk(pgd, _stext, _etext, PAGE_KERNEL_EXEC);
+       map_kernel_chunk(pgd, _stext, __start_rodata, PAGE_KERNEL_EXEC);
+       map_kernel_chunk(pgd, __start_rodata, _etext, PAGE_KERNEL);
        map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC);
        map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);
 
--

2.4.3

Signature · 2 lines
Kees Cook
Chrome OS & Brillo Security
maintainer Applied
#2 lore raw

PATCH v3 1/1 arm64: mm: Mark .rodata as RO

In reply to Jeremy Linton
Quoted 5 lines · Jeremy Linton

Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. It also adds the .rodata section to the mem_init banner.

Signed-off-by Jeremy Linton <[email protected]>

Applied. Thanks.

Signature · 1 line
Catalin
reviewer
#3 lore raw

PATCH v3 1/1 arm64: mm: Mark .rodata as RO

In reply to Jeremy Linton
Quoted 16 lines · Jeremy Linton

Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. It also adds the .rodata section to the mem_init banner.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S |  5 +++--
 arch/arm64/mm/init.c            |  4 +++-
 arch/arm64/mm/mmu.c             | 17 +++++++++++++----
 3 files changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 8f4fc2c..9208f53 100644

arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 120

Quoted 9 lines
arch/arm64/kernel/vmlinux.lds.S +3 -2
@@ -114,8 +114,9 @@ SECTIONS
 		*(.got)			/* Global offset table		*/
 	}
 
-	RO_DATA(PAGE_SIZE)
-	EXCEPTION_TABLE(8)
+	ALIGN_DEBUG_RO_MIN(0)
+	RO_DATA(PAGE_SIZE)		/* everything from this point to */
+	EXCEPTION_TABLE(8)		/* _etext will be marked RO NX   */
 	NOTES

That should be ALIGN_DEBUG_RO_MIN(PAGE_SIZE), given we map .text and .rodata separately regardless of DEBUG_RODATA (and hence they need to never share a page).

You'll get away with it, since RO_DATA forces PAGE_SIZE alignment for __start_rodata, but for consistency that should be fixed up.

With that:

Acked-by Mark Rutland <[email protected]>

Catalin, I assume you can fix that up locally?

Mark.

author
#4 lore raw

PATCH v3 1/1 arm64: mm: Mark .rodata as RO

Quoted 20 lines · Mark Rutland
Quoted 19 lines · Jeremy Linton

Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. It also adds the .rodata section to the mem_init banner.

Signed-off-by Jeremy Linton <[email protected]>
---
  arch/arm64/kernel/vmlinux.lds.S |  5 +++--
  arch/arm64/mm/init.c            |  4 +++-
  arch/arm64/mm/mmu.c             | 17 +++++++++++++----
  3 files changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 8f4fc2c..9208f53 100644
@@ -114,8 +114,9 @@ SECTIONS
  		*(.got)			/* Global offset table		*/
  	}
Quoted 11 lines

arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 120

Quoted 6 lines
arch/arm64/kernel/vmlinux.lds.S +3 -2
-	RO_DATA(PAGE_SIZE)
-	EXCEPTION_TABLE(8)
+	ALIGN_DEBUG_RO_MIN(0)
+	RO_DATA(PAGE_SIZE)		/* everything from this point to */
+	EXCEPTION_TABLE(8)		/* _etext will be marked RO NX   */
  	NOTES

That should be ALIGN_DEBUG_RO_MIN(PAGE_SIZE), given we map .text and .rodata separately regardless of DEBUG_RODATA (and hence they need to never share a page).

The RO_DATA macro has an explicit alignment (PAGE_SIZE in this case) in it too. That is why I left it at 0 to make it clear that it wasn't changing the alignment unless DEBUG_RO was enabled (and in that case only really applies if the section_size/cont_size is enabled).