Skip to contentSkip to conversation

arm64: mm: Mark .rodata as RO

PATCH Superseded Jeremy Linton 3 participants 8 messages Feb 17, 2016 arm64

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jeremy Linton

author
#0 lore raw

v2 cover 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.

This patch is based on top of "PATCH v2 flag contiguous PTES in linear mapping"

V1->V2
Added .rodata to kernel VM range notice in mem_init Split _stext->_etext into an additional chunk in map_kernel

Jeremy Linton (2):
arm64: mm: Mark .rodata as RO
arm64/mm: Add .rodata to kernel virtual memory boot notice

 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(-)
Signature · 1 line
2.4.3
author
#1 lore raw

PATCH v2 1/2 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.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S |  5 +++--
 arch/arm64/mm/mmu.c             | 17 +++++++++++++----
 2 files changed, 16 insertions(+), 6 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/mmu.c +13 -4
index ab69a99..9aa5201 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_EXEC);
 	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
#2 lore raw

PATCH v2 1/2 arm64: mm: Mark .rodata as RO

Quoted 52 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.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S |  5 +++--
 arch/arm64/mm/mmu.c             | 17 +++++++++++++----
 2 files changed, 16 insertions(+), 6 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/mmu.c +11 -3
index ab69a99..9aa5201 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

arch/arm64/mm/mmu.c static void __init map_kernel_chunk(pgd_t *pgd, void *va_start, void *va_end, · line 498

Quoted 6 lines
arch/arm64/mm/mmu.c +2 -1
@@ -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_EXEC);

Couldn't we map this non-exec from the start?

Quoted 6 lines
arch/arm64/mm/mmu.c +0 -1
        map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC);
        map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);
 
--

2.4.3

author
#3 lore raw

PATCH v2 1/2 arm64: mm: Mark .rodata as RO

Quoted 4 lines · Ard Biesheuvel
Quoted 3 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.

(trimming)

Quoted 6 lines

arch/arm64/mm/mmu.c static void __init map_kernel_chunk(pgd_t *pgd, void *va_start, void *va_end, · line 498

Quoted 4 lines
arch/arm64/mm/mmu.c +2 -1
-       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_EXEC);

Couldn't we map this non-exec from the start?

Probably, Mark suggested that, but Kees seemed to have reasons not to. Either way, my opinion is that for that change to make sense we also need to always enable the functionality turned on by DEBUG_RODATA.

Quoted 8 lines
Quoted 7 lines
         map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC);
         map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);

--
2.4.3

#4 lore raw

PATCH v2 1/2 arm64: mm: Mark .rodata as RO

Quoted 17 lines · Jeremy Linton
Quoted 8 lines · Ard Biesheuvel
Quoted 6 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.

(trimming)

Quoted 7 lines
Quoted 5 lines
arch/arm64/mm/mmu.c +2 -1
-       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_EXEC);
Quoted 7 lines
Quoted 1 line

Couldn't we map this non-exec from the start?

Probably, Mark suggested that, but Kees seemed to have reasons not to. Either way, my opinion is that for that change to make sense we also need to always enable the functionality turned on by DEBUG_RODATA.

Actually, I think that was about mapping read-only, not non-exec. For text patching and Kees's __ro_after_init stuff, the region would need to be writable early on. But I don't think there is a reason to make it executable.

reviewer
#5 lore raw

PATCH v2 1/2 arm64: mm: Mark .rodata as RO

Quoted 26 lines · Ard Biesheuvel
Quoted 17 lines · Jeremy Linton
Quoted 8 lines · Ard Biesheuvel
Quoted 6 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.

(trimming)

Quoted 7 lines
Quoted 5 lines
arch/arm64/mm/mmu.c +2 -1
-       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_EXEC);
Quoted 7 lines
Quoted 1 line

Couldn't we map this non-exec from the start?

Probably, Mark suggested that, but Kees seemed to have reasons not to. Either way, my opinion is that for that change to make sense we also need to always enable the functionality turned on by DEBUG_RODATA.

Quoted 4 lines

Actually, I think that was about mapping read-only, not non-exec. For text patching and Kees's __ro_after_init stuff, the region would need to be writable early on. But I don't think there is a reason to make it executable.

Yup, we should be able to make it PAGE_KERNEL here, even if we can't make it PAGE_KERNEL_RO.

Mark.

author
#6 lore raw

PATCH v2 2/2 arm64/mm: Add .rodata to kernel virtual memory boot notice

In reply to Jeremy Linton

Now that .rodata has differing permissions from .text display it in the mem_init banner as a seperate memory range.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/mm/init.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
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
Signature · 1 line
2.4.3
reviewer
#7 lore raw

PATCH v2 2/2 arm64/mm: Add .rodata to kernel virtual memory boot notice

Quoted 7 lines · Jeremy Linton

Now that .rodata has differing permissions from .text display it in the mem_init banner as a seperate memory range.

Signed-off-by Jeremy Linton <[email protected]>
---
 arch/arm64/mm/init.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

This looks fine, but I think it would be better to have this folded into the prior patch.

Mark.

Quoted 26 lines
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
Signature · 1 line
2.4.3