linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Improve kernel section protections
@ 2020-11-05  0:04 Atish Patra
  2020-11-05  0:04 ` [PATCH v3 1/5] RISC-V: Move __start_kernel to .head.text Atish Patra
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

This series aims at improving kernel permissions by doing following things.

1. Protect kernel sections early instead of after /init.
2. Protect .init.text & .init.data sections with appropriate permissions.
3. Move dynamic relocation section to _init.
4. Moved .init sections after .text. This is what most of the other archs
   are also doing.

After applying this patch, here are the linear mapped sections with non-uefi boot.

---[ Linear mapping ]---
0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PMD     D A . . X . R V
0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PMD     D A . . . W R V
0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PMD     D A . . . . R V
0xffffffe001200000-0xffffffe03fe00000    0x0000000081400000      1004M PMD     D A . . . W R V

Linear mapping with uefi boot.

---[ Linear mapping ]---
0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PTE     D A . . X . R V
0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PTE     D A . . . W R V
0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PTE     D A . . . . R V
0xffffffe001200000-0xffffffe03e534000    0x0000000081400000   1002704K PTE     D A . . . W R V
0xffffffe03e538000-0xffffffe03e539000    0x00000000be738000         4K PTE     D A . . . W R V
0xffffffe03e53a000-0xffffffe03e53c000    0x00000000be73a000         8K PTE     D A . . . W R V
0xffffffe03e540000-0xffffffe03e541000    0x00000000be740000         4K PTE     D A . . . W R V
0xffffffe03e545000-0xffffffe03e546000    0x00000000be745000         4K PTE     D A . . . W R V
0xffffffe03e549000-0xffffffe03e54a000    0x00000000be749000         4K PTE     D A . . . W R V
0xffffffe03e54b000-0xffffffe03fd6d000    0x00000000be74b000     24712K PTE     D A . . . W R V
0xffffffe03fd6e000-0xffffffe03fdee000    0x00000000bff6e000       512K PTE     D A . . . W R V


Changes from v2->v3:
1. Added few extra comments to clarify rodata permissions.
2. Changed the name of the functions set_memory_default to set_memory_rw_nx.
3. Squashed patch 3&5 together as they depend on each other to allow
   bisectability.
4. Removed redundant arguments in protect_kernel_text_data.

Changes from v1->v2:
1. .init.text section is aligned with SECTION_ALIGN.
2. .init.text is moved to below of .text so that .head.text & .text are in
   one section.
3. We don't need Guo's fix for static object issue.
4. Rebased on 5.10-rc1.

Atish Patra (5):
RISC-V: Move __start_kernel to .head.text
RISC-V: Initialize SBI early
RISC-V: Align the .init.text section
RISC-V: Protect all kernel sections including init early
RISC-V: Move dynamic relocation section under __init

arch/riscv/include/asm/sections.h   |  2 +
arch/riscv/include/asm/set_memory.h |  4 ++
arch/riscv/kernel/head.S            |  1 -
arch/riscv/kernel/setup.c           | 19 +++++++--
arch/riscv/kernel/vmlinux.lds.S     | 63 +++++++++++++++++------------
arch/riscv/mm/init.c                | 21 +++++++---
arch/riscv/mm/pageattr.c            |  6 +++
7 files changed, 80 insertions(+), 36 deletions(-)

--
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/5] RISC-V: Move __start_kernel to .head.text
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
@ 2020-11-05  0:04 ` Atish Patra
  2020-11-05  0:04 ` [PATCH v3 2/5] RISC-V: Initialize SBI early Atish Patra
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

Currently, __start_kernel is kept in _init while _start is in head section.
This may result in "relocation truncated to fit error" if _init section is
moved far from head. It also makes sense to keep entire head.S in one
section.

Keep __start_kernel in head section rather than _init.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/head.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 11e2a4fe66e0..45dbdae930bf 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -177,7 +177,6 @@ setup_trap_vector:
 
 END(_start)
 
-	__INIT
 ENTRY(_start_kernel)
 	/* Mask all interrupts */
 	csrw CSR_IE, zero
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/5] RISC-V: Initialize SBI early
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
  2020-11-05  0:04 ` [PATCH v3 1/5] RISC-V: Move __start_kernel to .head.text Atish Patra
@ 2020-11-05  0:04 ` Atish Patra
  2020-11-05  0:04 ` [PATCH v3 3/5] RISC-V: Align the .init.text section Atish Patra
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

Currently, SBI is initialized towards the end of arch setup. This prevents
the set memory operations to be invoked earlier as it requires a full tlb
flush.

Initialize SBI as early as possible.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/setup.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index c424cc6dd833..b916f4a904c4 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -89,6 +89,9 @@ void __init setup_arch(char **cmdline_p)
 		pr_err("No DTB found in kernel mappings\n");
 #endif
 
+	if (IS_ENABLED(CONFIG_RISCV_SBI))
+		sbi_init();
+
 #ifdef CONFIG_SWIOTLB
 	swiotlb_init(1);
 #endif
@@ -97,10 +100,6 @@ void __init setup_arch(char **cmdline_p)
 	kasan_init();
 #endif
 
-#if IS_ENABLED(CONFIG_RISCV_SBI)
-	sbi_init();
-#endif
-
 #ifdef CONFIG_SMP
 	setup_smp();
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
  2020-11-05  0:04 ` [PATCH v3 1/5] RISC-V: Move __start_kernel to .head.text Atish Patra
  2020-11-05  0:04 ` [PATCH v3 2/5] RISC-V: Initialize SBI early Atish Patra
@ 2020-11-05  0:04 ` Atish Patra
  2020-12-16  6:02   ` Palmer Dabbelt
  2020-11-05  0:04 ` [PATCH v3 4/5] RISC-V: Protect all kernel sections including init early Atish Patra
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Jim Wilson, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

In order to improve kernel text protection, we need separate .init.text/
.init.data/.text in separate sections. However, RISC-V linker relaxation
code is not aware of any alignment between sections. As a result, it may
relax any RISCV_CALL relocations between sections to JAL without realizing
that an inter section alignment may move the address farther. That may
lead to a relocation truncated fit error. However, linker relaxation code
is aware of the individual section alignments.

The detailed discussion on this issue can be found here.
https://github.com/riscv/riscv-gnu-toolchain/issues/738

Keep the .init.text section aligned so that linker relaxation will take
that as a hint while relaxing inter section calls.
Here are the code size changes for each section because of this change.

section         change in size (in bytes)
  .head.text      +4
  .text           +40
  .init.text      +6530
  .exit.text      +84

The only significant increase in size happened for .init.text because
all intra relocations also use 2MB alignment.

Suggested-by: Jim Wilson <jimw@sifive.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index 3ffbd6cbdb86..cacd7898ba7f 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -30,7 +30,13 @@ SECTIONS
 	. = ALIGN(PAGE_SIZE);
 
 	__init_begin = .;
-	INIT_TEXT_SECTION(PAGE_SIZE)
+	__init_text_begin = .;
+	.init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
+		_sinittext = .;						\
+		INIT_TEXT						\
+		_einittext = .;						\
+	}
+
 	. = ALIGN(8);
 	__soc_early_init_table : {
 		__soc_early_init_table_start = .;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/5] RISC-V: Protect all kernel sections including init early
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
                   ` (2 preceding siblings ...)
  2020-11-05  0:04 ` [PATCH v3 3/5] RISC-V: Align the .init.text section Atish Patra
@ 2020-11-05  0:04 ` Atish Patra
  2020-11-05  0:04 ` [PATCH v3 5/5] RISC-V: Move dynamic relocation section under __init Atish Patra
  2020-11-24  7:21 ` [PATCH v3 0/5] Improve kernel section protections Greentime Hu
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

Currently, .init.text & .init.data are intermixed which makes it impossible
apply different permissions to them. .init.data shouldn't need exec
permissions while .init.text shouldn't have write permission. Moreover,
the strict permission are only enforced /init starts. This leaves the
kernel vulnerable from possible buggy built-in modules.

Keep .init.text & .data in separate sections so that different permissions
are applied to each section. Apply permissions to individual sections as
early as possible. This improves the kernel protection under
CONFIG_STRICT_KERNEL_RWX. We also need to restore the permissions for the
entire _init section after it is freed so that those pages can be used
for other purpose.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/include/asm/sections.h   |  2 ++
 arch/riscv/include/asm/set_memory.h |  4 +++
 arch/riscv/kernel/setup.c           | 12 +++++++
 arch/riscv/kernel/vmlinux.lds.S     | 49 ++++++++++++++++-------------
 arch/riscv/mm/init.c                | 21 ++++++++++---
 arch/riscv/mm/pageattr.c            |  6 ++++
 6 files changed, 67 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
index 3a9971b1210f..1595c5b60cfd 100644
--- a/arch/riscv/include/asm/sections.h
+++ b/arch/riscv/include/asm/sections.h
@@ -9,5 +9,7 @@
 
 extern char _start[];
 extern char _start_kernel[];
+extern char __init_data_begin[], __init_data_end[];
+extern char __init_text_begin[], __init_text_end[];
 
 #endif /* __ASM_SECTIONS_H */
diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
index 4c5bae7ca01c..b21f4bea6434 100644
--- a/arch/riscv/include/asm/set_memory.h
+++ b/arch/riscv/include/asm/set_memory.h
@@ -15,11 +15,15 @@ int set_memory_ro(unsigned long addr, int numpages);
 int set_memory_rw(unsigned long addr, int numpages);
 int set_memory_x(unsigned long addr, int numpages);
 int set_memory_nx(unsigned long addr, int numpages);
+int set_memory_rw_nx(unsigned long addr, int numpages);
+void protect_kernel_text_data(void);
 #else
 static inline int set_memory_ro(unsigned long addr, int numpages) { return 0; }
 static inline int set_memory_rw(unsigned long addr, int numpages) { return 0; }
 static inline int set_memory_x(unsigned long addr, int numpages) { return 0; }
 static inline int set_memory_nx(unsigned long addr, int numpages) { return 0; }
+static inline void protect_kernel_text_data(void) {};
+static inline int set_memory_rw_nx(unsigned long addr, int numpages) { return 0; }
 #endif
 
 int set_direct_map_invalid_noflush(struct page *page);
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index b916f4a904c4..0cbbf62c7747 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -22,6 +22,7 @@
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
 #include <asm/setup.h>
+#include <asm/set_memory.h>
 #include <asm/sections.h>
 #include <asm/sbi.h>
 #include <asm/tlbflush.h>
@@ -92,6 +93,8 @@ void __init setup_arch(char **cmdline_p)
 	if (IS_ENABLED(CONFIG_RISCV_SBI))
 		sbi_init();
 
+	if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+		protect_kernel_text_data();
 #ifdef CONFIG_SWIOTLB
 	swiotlb_init(1);
 #endif
@@ -121,3 +124,12 @@ static int __init topology_init(void)
 	return 0;
 }
 subsys_initcall(topology_init);
+
+void free_initmem(void)
+{
+	unsigned long init_begin = (unsigned long)__init_begin;
+	unsigned long init_end = (unsigned long)__init_end;
+
+	set_memory_rw_nx(init_begin, (init_end - init_begin) >> PAGE_SHIFT);
+	free_initmem_default(POISON_FREE_INITMEM);
+}
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index cacd7898ba7f..ca19ee5acd0a 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -29,6 +29,22 @@ SECTIONS
 	HEAD_TEXT_SECTION
 	. = ALIGN(PAGE_SIZE);
 
+	.text : {
+		_text = .;
+		_stext = .;
+		TEXT_TEXT
+		SCHED_TEXT
+		CPUIDLE_TEXT
+		LOCK_TEXT
+		KPROBES_TEXT
+		ENTRY_TEXT
+		IRQENTRY_TEXT
+		SOFTIRQENTRY_TEXT
+		*(.fixup)
+		_etext = .;
+	}
+
+	. = ALIGN(SECTION_ALIGN);
 	__init_begin = .;
 	__init_text_begin = .;
 	.init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
@@ -53,35 +69,24 @@ SECTIONS
 	{
 		EXIT_TEXT
 	}
-	.exit.data :
-	{
-		EXIT_DATA
-	}
-	PERCPU_SECTION(L1_CACHE_BYTES)
-	__init_end = .;
 
+	__init_text_end = .;
 	. = ALIGN(SECTION_ALIGN);
-	.text : {
-		_text = .;
-		_stext = .;
-		TEXT_TEXT
-		SCHED_TEXT
-		CPUIDLE_TEXT
-		LOCK_TEXT
-		KPROBES_TEXT
-		ENTRY_TEXT
-		IRQENTRY_TEXT
-		SOFTIRQENTRY_TEXT
-		*(.fixup)
-		_etext = .;
-	}
-
 #ifdef CONFIG_EFI
 	. = ALIGN(PECOFF_SECTION_ALIGNMENT);
 	__pecoff_text_end = .;
 #endif
-
+	/* Start of init data section */
+	__init_data_begin = .;
 	INIT_DATA_SECTION(16)
+	.exit.data :
+	{
+		EXIT_DATA
+	}
+	PERCPU_SECTION(L1_CACHE_BYTES)
+
+	__init_data_end = .;
+	__init_end = .;
 
 	/* Start of data section */
 	_sdata = .;
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 0d13d0c36a7d..00793f3c837e 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -622,18 +622,29 @@ static inline void setup_vm_final(void)
 #endif /* CONFIG_MMU */
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
-void mark_rodata_ro(void)
+void protect_kernel_text_data(void)
 {
-	unsigned long text_start = (unsigned long)_text;
-	unsigned long text_end = (unsigned long)_etext;
+	unsigned long text_start = (unsigned long)_start;
+	unsigned long init_text_start = (unsigned long)__init_text_begin;
+	unsigned long init_data_start = (unsigned long)__init_data_begin;
 	unsigned long rodata_start = (unsigned long)__start_rodata;
 	unsigned long data_start = (unsigned long)_data;
 	unsigned long max_low = (unsigned long)(__va(PFN_PHYS(max_low_pfn)));
 
-	set_memory_ro(text_start, (text_end - text_start) >> PAGE_SHIFT);
-	set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
+	set_memory_ro(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
+	set_memory_ro(init_text_start, (init_data_start - init_text_start) >> PAGE_SHIFT);
+	set_memory_nx(init_data_start, (rodata_start - init_data_start) >> PAGE_SHIFT);
+	/* rodata section is marked readonly in mark_rodata_ro */
 	set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
 	set_memory_nx(data_start, (max_low - data_start) >> PAGE_SHIFT);
+}
+
+void mark_rodata_ro(void)
+{
+	unsigned long rodata_start = (unsigned long)__start_rodata;
+	unsigned long data_start = (unsigned long)_data;
+
+	set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
 
 	debug_checkwx();
 }
diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
index 19fecb362d81..c64ec224fd0c 100644
--- a/arch/riscv/mm/pageattr.c
+++ b/arch/riscv/mm/pageattr.c
@@ -128,6 +128,12 @@ static int __set_memory(unsigned long addr, int numpages, pgprot_t set_mask,
 	return ret;
 }
 
+int set_memory_rw_nx(unsigned long addr, int numpages)
+{
+	return __set_memory(addr, numpages, __pgprot(_PAGE_READ | _PAGE_WRITE),
+			    __pgprot(_PAGE_EXEC));
+}
+
 int set_memory_ro(unsigned long addr, int numpages)
 {
 	return __set_memory(addr, numpages, __pgprot(_PAGE_READ),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 5/5] RISC-V: Move dynamic relocation section under __init
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
                   ` (3 preceding siblings ...)
  2020-11-05  0:04 ` [PATCH v3 4/5] RISC-V: Protect all kernel sections including init early Atish Patra
@ 2020-11-05  0:04 ` Atish Patra
  2020-11-24  7:21 ` [PATCH v3 0/5] Improve kernel section protections Greentime Hu
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-11-05  0:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Atish Patra, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Greentime Hu, Guo Ren, linux-riscv,
	Michel Lespinasse, Miguel Ojeda, Mike Rapoport, Palmer Dabbelt,
	Paul Walmsley, Zong Li

Dynamic relocation section are only required during boot. Those sections
can be freed after init. Thus, it can be moved to __init section.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/vmlinux.lds.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index ca19ee5acd0a..de03cb22d0e9 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -85,6 +85,10 @@ SECTIONS
 	}
 	PERCPU_SECTION(L1_CACHE_BYTES)
 
+	.rel.dyn : {
+		*(.rel.dyn*)
+	}
+
 	__init_data_end = .;
 	__init_end = .;
 
@@ -116,10 +120,6 @@ SECTIONS
 
 	BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
 
-	.rel.dyn : {
-		*(.rel.dyn*)
-	}
-
 #ifdef CONFIG_EFI
 	. = ALIGN(PECOFF_SECTION_ALIGNMENT);
 	__pecoff_data_virt_size = ABSOLUTE(. - __pecoff_text_end);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/5] Improve kernel section protections
  2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
                   ` (4 preceding siblings ...)
  2020-11-05  0:04 ` [PATCH v3 5/5] RISC-V: Move dynamic relocation section under __init Atish Patra
@ 2020-11-24  7:21 ` Greentime Hu
  2020-11-26  0:07   ` Palmer Dabbelt
  5 siblings, 1 reply; 13+ messages in thread
From: Greentime Hu @ 2020-11-24  7:21 UTC (permalink / raw)
  To: Atish Patra
  Cc: Linux Kernel Mailing List, Albert Ou, Andrew Morton, Anup Patel,
	Ard Biesheuvel, Guo Ren, linux-riscv, Michel Lespinasse,
	Miguel Ojeda, Mike Rapoport, Palmer Dabbelt, Paul Walmsley,
	Zong Li

Atish Patra <atish.patra@wdc.com> 於 2020年11月5日 週四 上午8:05寫道:
>
> This series aims at improving kernel permissions by doing following things.
>
> 1. Protect kernel sections early instead of after /init.
> 2. Protect .init.text & .init.data sections with appropriate permissions.
> 3. Move dynamic relocation section to _init.
> 4. Moved .init sections after .text. This is what most of the other archs
>    are also doing.
>
> After applying this patch, here are the linear mapped sections with non-uefi boot.
>
> ---[ Linear mapping ]---
> 0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PMD     D A . . X . R V
> 0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PMD     D A . . . W R V
> 0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PMD     D A . . . . R V
> 0xffffffe001200000-0xffffffe03fe00000    0x0000000081400000      1004M PMD     D A . . . W R V
>
> Linear mapping with uefi boot.
>
> ---[ Linear mapping ]---
> 0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PTE     D A . . X . R V
> 0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PTE     D A . . . W R V
> 0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PTE     D A . . . . R V
> 0xffffffe001200000-0xffffffe03e534000    0x0000000081400000   1002704K PTE     D A . . . W R V
> 0xffffffe03e538000-0xffffffe03e539000    0x00000000be738000         4K PTE     D A . . . W R V
> 0xffffffe03e53a000-0xffffffe03e53c000    0x00000000be73a000         8K PTE     D A . . . W R V
> 0xffffffe03e540000-0xffffffe03e541000    0x00000000be740000         4K PTE     D A . . . W R V
> 0xffffffe03e545000-0xffffffe03e546000    0x00000000be745000         4K PTE     D A . . . W R V
> 0xffffffe03e549000-0xffffffe03e54a000    0x00000000be749000         4K PTE     D A . . . W R V
> 0xffffffe03e54b000-0xffffffe03fd6d000    0x00000000be74b000     24712K PTE     D A . . . W R V
> 0xffffffe03fd6e000-0xffffffe03fdee000    0x00000000bff6e000       512K PTE     D A . . . W R V
>
>
> Changes from v2->v3:
> 1. Added few extra comments to clarify rodata permissions.
> 2. Changed the name of the functions set_memory_default to set_memory_rw_nx.
> 3. Squashed patch 3&5 together as they depend on each other to allow
>    bisectability.
> 4. Removed redundant arguments in protect_kernel_text_data.
>
> Changes from v1->v2:
> 1. .init.text section is aligned with SECTION_ALIGN.
> 2. .init.text is moved to below of .text so that .head.text & .text are in
>    one section.
> 3. We don't need Guo's fix for static object issue.
> 4. Rebased on 5.10-rc1.
>
> Atish Patra (5):
> RISC-V: Move __start_kernel to .head.text
> RISC-V: Initialize SBI early
> RISC-V: Align the .init.text section
> RISC-V: Protect all kernel sections including init early
> RISC-V: Move dynamic relocation section under __init
>
> arch/riscv/include/asm/sections.h   |  2 +
> arch/riscv/include/asm/set_memory.h |  4 ++
> arch/riscv/kernel/head.S            |  1 -
> arch/riscv/kernel/setup.c           | 19 +++++++--
> arch/riscv/kernel/vmlinux.lds.S     | 63 +++++++++++++++++------------
> arch/riscv/mm/init.c                | 21 +++++++---
> arch/riscv/mm/pageattr.c            |  6 +++
> 7 files changed, 80 insertions(+), 36 deletions(-)
>

Test this series in v5.10-rc3 in Qemu and it works.
Tested-by: Greentime Hu <greentime.hu@sifive.com>

Thank you. :)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/5] Improve kernel section protections
  2020-11-24  7:21 ` [PATCH v3 0/5] Improve kernel section protections Greentime Hu
@ 2020-11-26  0:07   ` Palmer Dabbelt
  0 siblings, 0 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2020-11-26  0:07 UTC (permalink / raw)
  To: greentime.hu
  Cc: Atish Patra, linux-kernel, aou, akpm, anup, ardb, ren_guo,
	linux-riscv, walken, ojeda, rppt, Paul Walmsley, zong.li

On Mon, 23 Nov 2020 23:21:08 PST (-0800), greentime.hu@sifive.com wrote:
> Atish Patra <atish.patra@wdc.com> 於 2020年11月5日 週四 上午8:05寫道:
>>
>> This series aims at improving kernel permissions by doing following things.
>>
>> 1. Protect kernel sections early instead of after /init.
>> 2. Protect .init.text & .init.data sections with appropriate permissions.
>> 3. Move dynamic relocation section to _init.
>> 4. Moved .init sections after .text. This is what most of the other archs
>>    are also doing.
>>
>> After applying this patch, here are the linear mapped sections with non-uefi boot.
>>
>> ---[ Linear mapping ]---
>> 0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PMD     D A . . X . R V
>> 0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PMD     D A . . . W R V
>> 0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PMD     D A . . . . R V
>> 0xffffffe001200000-0xffffffe03fe00000    0x0000000081400000      1004M PMD     D A . . . W R V
>>
>> Linear mapping with uefi boot.
>>
>> ---[ Linear mapping ]---
>> 0xffffffe000000000-0xffffffe000800000    0x0000000080200000         8M PTE     D A . . X . R V
>> 0xffffffe000800000-0xffffffe000c00000    0x0000000080a00000         4M PTE     D A . . . W R V
>> 0xffffffe000c00000-0xffffffe001200000    0x0000000080e00000         6M PTE     D A . . . . R V
>> 0xffffffe001200000-0xffffffe03e534000    0x0000000081400000   1002704K PTE     D A . . . W R V
>> 0xffffffe03e538000-0xffffffe03e539000    0x00000000be738000         4K PTE     D A . . . W R V
>> 0xffffffe03e53a000-0xffffffe03e53c000    0x00000000be73a000         8K PTE     D A . . . W R V
>> 0xffffffe03e540000-0xffffffe03e541000    0x00000000be740000         4K PTE     D A . . . W R V
>> 0xffffffe03e545000-0xffffffe03e546000    0x00000000be745000         4K PTE     D A . . . W R V
>> 0xffffffe03e549000-0xffffffe03e54a000    0x00000000be749000         4K PTE     D A . . . W R V
>> 0xffffffe03e54b000-0xffffffe03fd6d000    0x00000000be74b000     24712K PTE     D A . . . W R V
>> 0xffffffe03fd6e000-0xffffffe03fdee000    0x00000000bff6e000       512K PTE     D A . . . W R V
>>
>>
>> Changes from v2->v3:
>> 1. Added few extra comments to clarify rodata permissions.
>> 2. Changed the name of the functions set_memory_default to set_memory_rw_nx.
>> 3. Squashed patch 3&5 together as they depend on each other to allow
>>    bisectability.
>> 4. Removed redundant arguments in protect_kernel_text_data.
>>
>> Changes from v1->v2:
>> 1. .init.text section is aligned with SECTION_ALIGN.
>> 2. .init.text is moved to below of .text so that .head.text & .text are in
>>    one section.
>> 3. We don't need Guo's fix for static object issue.
>> 4. Rebased on 5.10-rc1.
>>
>> Atish Patra (5):
>> RISC-V: Move __start_kernel to .head.text
>> RISC-V: Initialize SBI early
>> RISC-V: Align the .init.text section
>> RISC-V: Protect all kernel sections including init early
>> RISC-V: Move dynamic relocation section under __init
>>
>> arch/riscv/include/asm/sections.h   |  2 +
>> arch/riscv/include/asm/set_memory.h |  4 ++
>> arch/riscv/kernel/head.S            |  1 -
>> arch/riscv/kernel/setup.c           | 19 +++++++--
>> arch/riscv/kernel/vmlinux.lds.S     | 63 +++++++++++++++++------------
>> arch/riscv/mm/init.c                | 21 +++++++---
>> arch/riscv/mm/pageattr.c            |  6 +++
>> 7 files changed, 80 insertions(+), 36 deletions(-)
>>
>
> Test this series in v5.10-rc3 in Qemu and it works.
> Tested-by: Greentime Hu <greentime.hu@sifive.com>
>
> Thank you. :)

Thanks, this is on for-next.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-11-05  0:04 ` [PATCH v3 3/5] RISC-V: Align the .init.text section Atish Patra
@ 2020-12-16  6:02   ` Palmer Dabbelt
  2020-12-17  6:51     ` Palmer Dabbelt
  0 siblings, 1 reply; 13+ messages in thread
From: Palmer Dabbelt @ 2020-12-16  6:02 UTC (permalink / raw)
  To: Atish Patra
  Cc: linux-kernel, Atish Patra, Jim Wilson, aou, akpm, anup, ardb,
	greentime.hu, ren_guo, linux-riscv, walken, ojeda, rppt,
	Paul Walmsley, zong.li

On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
> In order to improve kernel text protection, we need separate .init.text/
> .init.data/.text in separate sections. However, RISC-V linker relaxation
> code is not aware of any alignment between sections. As a result, it may
> relax any RISCV_CALL relocations between sections to JAL without realizing
> that an inter section alignment may move the address farther. That may
> lead to a relocation truncated fit error. However, linker relaxation code
> is aware of the individual section alignments.
>
> The detailed discussion on this issue can be found here.
> https://github.com/riscv/riscv-gnu-toolchain/issues/738
>
> Keep the .init.text section aligned so that linker relaxation will take
> that as a hint while relaxing inter section calls.
> Here are the code size changes for each section because of this change.
>
> section         change in size (in bytes)
>   .head.text      +4
>   .text           +40
>   .init.text      +6530
>   .exit.text      +84
>
> The only significant increase in size happened for .init.text because
> all intra relocations also use 2MB alignment.
>
> Suggested-by: Jim Wilson <jimw@sifive.com>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index 3ffbd6cbdb86..cacd7898ba7f 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -30,7 +30,13 @@ SECTIONS
>  	. = ALIGN(PAGE_SIZE);
>
>  	__init_begin = .;
> -	INIT_TEXT_SECTION(PAGE_SIZE)
> +	__init_text_begin = .;
> +	.init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
> +		_sinittext = .;						\
> +		INIT_TEXT						\
> +		_einittext = .;						\
> +	}
> +
>  	. = ALIGN(8);
>  	__soc_early_init_table : {
>  		__soc_early_init_table_start = .;

Not sure what's going on here (or why I wasn't catching it earlier), but this
is breaking boot on one of my test configs.  I'm not getting any Linux boot
spew, so it's something fairly early.  I'm running defconfig with

    CONFIG_PREEMPT=y
    CONFIG_DEBUG_PREEMPT=y
    CONFIG_PROVE_LOCKING=y

It looks like that's been throwing a bunch of warnings for a while, but it did
at least used to boot.  No idea what PREEMPT would have to do with this, and
the other two don't generally trigger issues that early in boot (or at least,
trigger halts that early in boot).

There's a bunch of other stuff that depends on this that's on for-next so I
don't want to just drop it, but I also don't want to break something.  I'm just
running QEMU's virt board.

I'll take a look again tomorrow night, but if anyone has some time to look
that'd be great!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-12-16  6:02   ` Palmer Dabbelt
@ 2020-12-17  6:51     ` Palmer Dabbelt
  2020-12-17  8:33       ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Palmer Dabbelt @ 2020-12-17  6:51 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Atish Patra, linux-kernel, Atish Patra, Jim Wilson, aou, akpm,
	anup, ardb, greentime.hu, ren_guo, linux-riscv, walken, ojeda,
	rppt, Paul Walmsley, zong.li

On Tue, 15 Dec 2020 22:02:54 PST (-0800), Palmer Dabbelt wrote:
> On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
>> In order to improve kernel text protection, we need separate .init.text/
>> .init.data/.text in separate sections. However, RISC-V linker relaxation
>> code is not aware of any alignment between sections. As a result, it may
>> relax any RISCV_CALL relocations between sections to JAL without realizing
>> that an inter section alignment may move the address farther. That may
>> lead to a relocation truncated fit error. However, linker relaxation code
>> is aware of the individual section alignments.
>>
>> The detailed discussion on this issue can be found here.
>> https://github.com/riscv/riscv-gnu-toolchain/issues/738
>>
>> Keep the .init.text section aligned so that linker relaxation will take
>> that as a hint while relaxing inter section calls.
>> Here are the code size changes for each section because of this change.
>>
>> section         change in size (in bytes)
>>   .head.text      +4
>>   .text           +40
>>   .init.text      +6530
>>   .exit.text      +84
>>
>> The only significant increase in size happened for .init.text because
>> all intra relocations also use 2MB alignment.
>>
>> Suggested-by: Jim Wilson <jimw@sifive.com>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> ---
>>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>> index 3ffbd6cbdb86..cacd7898ba7f 100644
>> --- a/arch/riscv/kernel/vmlinux.lds.S
>> +++ b/arch/riscv/kernel/vmlinux.lds.S
>> @@ -30,7 +30,13 @@ SECTIONS
>>  	. = ALIGN(PAGE_SIZE);
>>
>>  	__init_begin = .;
>> -	INIT_TEXT_SECTION(PAGE_SIZE)
>> +	__init_text_begin = .;
>> +	.init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
>> +		_sinittext = .;						\
>> +		INIT_TEXT						\
>> +		_einittext = .;						\
>> +	}
>> +
>>  	. = ALIGN(8);
>>  	__soc_early_init_table : {
>>  		__soc_early_init_table_start = .;
>
> Not sure what's going on here (or why I wasn't catching it earlier), but this
> is breaking boot on one of my test configs.  I'm not getting any Linux boot
> spew, so it's something fairly early.  I'm running defconfig with
>
>     CONFIG_PREEMPT=y
>     CONFIG_DEBUG_PREEMPT=y
>     CONFIG_PROVE_LOCKING=y
>
> It looks like that's been throwing a bunch of warnings for a while, but it did
> at least used to boot.  No idea what PREEMPT would have to do with this, and
> the other two don't generally trigger issues that early in boot (or at least,
> trigger halts that early in boot).
>
> There's a bunch of other stuff that depends on this that's on for-next so I
> don't want to just drop it, but I also don't want to break something.  I'm just
> running QEMU's virt board.
>
> I'll take a look again tomorrow night, but if anyone has some time to look
> that'd be great!

Looks like this breaks on QEMU 5.0.0 but works on 5.2.0.  I guess technically
that means could be considered a regression, but as we don't really have any
scheme for which old versions of QEMU we support it's not absolute.  I'd
usually err on the side of keeping support for older platforms, but in this
case it's probably just not worth the time so I'm going to just ignore it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-12-17  6:51     ` Palmer Dabbelt
@ 2020-12-17  8:33       ` Atish Patra
  2020-12-18  8:19         ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-12-17  8:33 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, Albert Ou, Zong Li, Anup Patel,
	linux-kernel@vger.kernel.org List, linux-riscv, Atish Patra,
	ren_guo, Paul Walmsley, ojeda, Greentime Hu, Andrew Morton,
	Michel Lespinasse, Ard Biesheuvel, Mike Rapoport, Jim Wilson

On Wed, Dec 16, 2020 at 10:51 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 15 Dec 2020 22:02:54 PST (-0800), Palmer Dabbelt wrote:
> > On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
> >> In order to improve kernel text protection, we need separate .init.text/
> >> .init.data/.text in separate sections. However, RISC-V linker relaxation
> >> code is not aware of any alignment between sections. As a result, it may
> >> relax any RISCV_CALL relocations between sections to JAL without realizing
> >> that an inter section alignment may move the address farther. That may
> >> lead to a relocation truncated fit error. However, linker relaxation code
> >> is aware of the individual section alignments.
> >>
> >> The detailed discussion on this issue can be found here.
> >> https://github.com/riscv/riscv-gnu-toolchain/issues/738
> >>
> >> Keep the .init.text section aligned so that linker relaxation will take
> >> that as a hint while relaxing inter section calls.
> >> Here are the code size changes for each section because of this change.
> >>
> >> section         change in size (in bytes)
> >>   .head.text      +4
> >>   .text           +40
> >>   .init.text      +6530
> >>   .exit.text      +84
> >>
> >> The only significant increase in size happened for .init.text because
> >> all intra relocations also use 2MB alignment.
> >>
> >> Suggested-by: Jim Wilson <jimw@sifive.com>
> >> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> >> ---
> >>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> >> index 3ffbd6cbdb86..cacd7898ba7f 100644
> >> --- a/arch/riscv/kernel/vmlinux.lds.S
> >> +++ b/arch/riscv/kernel/vmlinux.lds.S
> >> @@ -30,7 +30,13 @@ SECTIONS
> >>      . = ALIGN(PAGE_SIZE);
> >>
> >>      __init_begin = .;
> >> -    INIT_TEXT_SECTION(PAGE_SIZE)
> >> +    __init_text_begin = .;
> >> +    .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
> >> +            _sinittext = .;                                         \
> >> +            INIT_TEXT                                               \
> >> +            _einittext = .;                                         \
> >> +    }
> >> +
> >>      . = ALIGN(8);
> >>      __soc_early_init_table : {
> >>              __soc_early_init_table_start = .;
> >
> > Not sure what's going on here (or why I wasn't catching it earlier), but this
> > is breaking boot on one of my test configs.  I'm not getting any Linux boot
> > spew, so it's something fairly early.  I'm running defconfig with
> >
> >     CONFIG_PREEMPT=y
> >     CONFIG_DEBUG_PREEMPT=y
> >     CONFIG_PROVE_LOCKING=y
> >
> > It looks like that's been throwing a bunch of warnings for a while, but it did
> > at least used to boot.  No idea what PREEMPT would have to do with this, and
> > the other two don't generally trigger issues that early in boot (or at least,
> > trigger halts that early in boot).
> >
> > There's a bunch of other stuff that depends on this that's on for-next so I
> > don't want to just drop it, but I also don't want to break something.  I'm just
> > running QEMU's virt board.
> >

I just verified for-next on QEMU 5.2.0 for virt (RV32,64, nommu) and
sifive_u as well.
I will give it a try on unleashed tomorrow as well with the above
configs enabled.

> > I'll take a look again tomorrow night, but if anyone has some time to look
> > that'd be great!
>
> Looks like this breaks on QEMU 5.0.0 but works on 5.2.0.

I will take a look tomorrow to check the root cause.

I guess technically
> that means could be considered a regression, but as we don't really have any
> scheme for which old versions of QEMU we support it's not absolute.  I'd
> usually err on the side of keeping support for older platforms, but in this
> case it's probably just not worth the time so I'm going to just ignore it.
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-12-17  8:33       ` Atish Patra
@ 2020-12-18  8:19         ` Atish Patra
  2020-12-23  4:14           ` Palmer Dabbelt
  0 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-12-18  8:19 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, Albert Ou, Zong Li, Anup Patel,
	linux-kernel@vger.kernel.org List, linux-riscv, Atish Patra,
	ren_guo, Paul Walmsley, ojeda, Greentime Hu, Andrew Morton,
	Michel Lespinasse, Ard Biesheuvel, Mike Rapoport, Jim Wilson

On Thu, Dec 17, 2020 at 12:33 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Wed, Dec 16, 2020 at 10:51 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Tue, 15 Dec 2020 22:02:54 PST (-0800), Palmer Dabbelt wrote:
> > > On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
> > >> In order to improve kernel text protection, we need separate .init.text/
> > >> .init.data/.text in separate sections. However, RISC-V linker relaxation
> > >> code is not aware of any alignment between sections. As a result, it may
> > >> relax any RISCV_CALL relocations between sections to JAL without realizing
> > >> that an inter section alignment may move the address farther. That may
> > >> lead to a relocation truncated fit error. However, linker relaxation code
> > >> is aware of the individual section alignments.
> > >>
> > >> The detailed discussion on this issue can be found here.
> > >> https://github.com/riscv/riscv-gnu-toolchain/issues/738
> > >>
> > >> Keep the .init.text section aligned so that linker relaxation will take
> > >> that as a hint while relaxing inter section calls.
> > >> Here are the code size changes for each section because of this change.
> > >>
> > >> section         change in size (in bytes)
> > >>   .head.text      +4
> > >>   .text           +40
> > >>   .init.text      +6530
> > >>   .exit.text      +84
> > >>
> > >> The only significant increase in size happened for .init.text because
> > >> all intra relocations also use 2MB alignment.
> > >>
> > >> Suggested-by: Jim Wilson <jimw@sifive.com>
> > >> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > >> ---
> > >>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
> > >>  1 file changed, 7 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > >> index 3ffbd6cbdb86..cacd7898ba7f 100644
> > >> --- a/arch/riscv/kernel/vmlinux.lds.S
> > >> +++ b/arch/riscv/kernel/vmlinux.lds.S
> > >> @@ -30,7 +30,13 @@ SECTIONS
> > >>      . = ALIGN(PAGE_SIZE);
> > >>
> > >>      __init_begin = .;
> > >> -    INIT_TEXT_SECTION(PAGE_SIZE)
> > >> +    __init_text_begin = .;
> > >> +    .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
> > >> +            _sinittext = .;                                         \
> > >> +            INIT_TEXT                                               \
> > >> +            _einittext = .;                                         \
> > >> +    }
> > >> +
> > >>      . = ALIGN(8);
> > >>      __soc_early_init_table : {
> > >>              __soc_early_init_table_start = .;
> > >
> > > Not sure what's going on here (or why I wasn't catching it earlier), but this
> > > is breaking boot on one of my test configs.  I'm not getting any Linux boot
> > > spew, so it's something fairly early.  I'm running defconfig with
> > >
> > >     CONFIG_PREEMPT=y
> > >     CONFIG_DEBUG_PREEMPT=y
> > >     CONFIG_PROVE_LOCKING=y
> > >
> > > It looks like that's been throwing a bunch of warnings for a while, but it did
> > > at least used to boot.  No idea what PREEMPT would have to do with this, and
> > > the other two don't generally trigger issues that early in boot (or at least,
> > > trigger halts that early in boot).
> > >

I am able to reproduce this issue but with CONFIG_PROVE_LOCKING not
CONFIG_PREEMPT.
With CONFIG_PREEMPT, I see a bunch of warnings around smp_processor_id
but it boots even with 5.0.
If CONFIG_PROVE_LOCKING is enabled, I am not able to boot using 5.0.
However, 5.2.0 works fine.
I am going to take a look at the issue with 5.0 and PROVE_LOCKING.

The config preempt warnings are resolved by the following patch. I
have tested it in Qemu.

https://patchwork.kernel.org/project/linux-riscv/patch/20201116081238.44223-1-wangkefeng.wang@huawei.com/

> > > There's a bunch of other stuff that depends on this that's on for-next so I
> > > don't want to just drop it, but I also don't want to break something.  I'm just
> > > running QEMU's virt board.
> > >
>
> I just verified for-next on QEMU 5.2.0 for virt (RV32,64, nommu) and
> sifive_u as well.
> I will give it a try on unleashed tomorrow as well with the above
> configs enabled.
>
> > > I'll take a look again tomorrow night, but if anyone has some time to look
> > > that'd be great!
> >
> > Looks like this breaks on QEMU 5.0.0 but works on 5.2.0.
>
> I will take a look tomorrow to check the root cause.
>
> I guess technically
> > that means could be considered a regression, but as we don't really have any
> > scheme for which old versions of QEMU we support it's not absolute.  I'd
> > usually err on the side of keeping support for older platforms, but in this
> > case it's probably just not worth the time so I'm going to just ignore it.
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] RISC-V: Align the .init.text section
  2020-12-18  8:19         ` Atish Patra
@ 2020-12-23  4:14           ` Palmer Dabbelt
  0 siblings, 0 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2020-12-23  4:14 UTC (permalink / raw)
  To: atishp
  Cc: Alistair Francis, aou, zong.li, anup, linux-kernel, linux-riscv,
	Atish Patra, ren_guo, Paul Walmsley, ojeda, greentime.hu, akpm,
	walken, ardb, rppt, Jim Wilson

On Fri, 18 Dec 2020 00:19:09 PST (-0800), atishp@atishpatra.org wrote:
> On Thu, Dec 17, 2020 at 12:33 AM Atish Patra <atishp@atishpatra.org> wrote:
>>
>> On Wed, Dec 16, 2020 at 10:51 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> >
>> > On Tue, 15 Dec 2020 22:02:54 PST (-0800), Palmer Dabbelt wrote:
>> > > On Wed, 04 Nov 2020 16:04:37 PST (-0800), Atish Patra wrote:
>> > >> In order to improve kernel text protection, we need separate .init.text/
>> > >> .init.data/.text in separate sections. However, RISC-V linker relaxation
>> > >> code is not aware of any alignment between sections. As a result, it may
>> > >> relax any RISCV_CALL relocations between sections to JAL without realizing
>> > >> that an inter section alignment may move the address farther. That may
>> > >> lead to a relocation truncated fit error. However, linker relaxation code
>> > >> is aware of the individual section alignments.
>> > >>
>> > >> The detailed discussion on this issue can be found here.
>> > >> https://github.com/riscv/riscv-gnu-toolchain/issues/738
>> > >>
>> > >> Keep the .init.text section aligned so that linker relaxation will take
>> > >> that as a hint while relaxing inter section calls.
>> > >> Here are the code size changes for each section because of this change.
>> > >>
>> > >> section         change in size (in bytes)
>> > >>   .head.text      +4
>> > >>   .text           +40
>> > >>   .init.text      +6530
>> > >>   .exit.text      +84
>> > >>
>> > >> The only significant increase in size happened for .init.text because
>> > >> all intra relocations also use 2MB alignment.
>> > >>
>> > >> Suggested-by: Jim Wilson <jimw@sifive.com>
>> > >> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> > >> ---
>> > >>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++++-
>> > >>  1 file changed, 7 insertions(+), 1 deletion(-)
>> > >>
>> > >> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>> > >> index 3ffbd6cbdb86..cacd7898ba7f 100644
>> > >> --- a/arch/riscv/kernel/vmlinux.lds.S
>> > >> +++ b/arch/riscv/kernel/vmlinux.lds.S
>> > >> @@ -30,7 +30,13 @@ SECTIONS
>> > >>      . = ALIGN(PAGE_SIZE);
>> > >>
>> > >>      __init_begin = .;
>> > >> -    INIT_TEXT_SECTION(PAGE_SIZE)
>> > >> +    __init_text_begin = .;
>> > >> +    .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) ALIGN(SECTION_ALIGN) { \
>> > >> +            _sinittext = .;                                         \
>> > >> +            INIT_TEXT                                               \
>> > >> +            _einittext = .;                                         \
>> > >> +    }
>> > >> +
>> > >>      . = ALIGN(8);
>> > >>      __soc_early_init_table : {
>> > >>              __soc_early_init_table_start = .;
>> > >
>> > > Not sure what's going on here (or why I wasn't catching it earlier), but this
>> > > is breaking boot on one of my test configs.  I'm not getting any Linux boot
>> > > spew, so it's something fairly early.  I'm running defconfig with
>> > >
>> > >     CONFIG_PREEMPT=y
>> > >     CONFIG_DEBUG_PREEMPT=y
>> > >     CONFIG_PROVE_LOCKING=y
>> > >
>> > > It looks like that's been throwing a bunch of warnings for a while, but it did
>> > > at least used to boot.  No idea what PREEMPT would have to do with this, and
>> > > the other two don't generally trigger issues that early in boot (or at least,
>> > > trigger halts that early in boot).
>> > >
>
> I am able to reproduce this issue but with CONFIG_PROVE_LOCKING not
> CONFIG_PREEMPT.
> With CONFIG_PREEMPT, I see a bunch of warnings around smp_processor_id
> but it boots even with 5.0.
> If CONFIG_PROVE_LOCKING is enabled, I am not able to boot using 5.0.
> However, 5.2.0 works fine.
> I am going to take a look at the issue with 5.0 and PROVE_LOCKING.
>
> The config preempt warnings are resolved by the following patch. I
> have tested it in Qemu.
>
> https://patchwork.kernel.org/project/linux-riscv/patch/20201116081238.44223-1-wangkefeng.wang@huawei.com/

Thanks!

>
>> > > There's a bunch of other stuff that depends on this that's on for-next so I
>> > > don't want to just drop it, but I also don't want to break something.  I'm just
>> > > running QEMU's virt board.
>> > >
>>
>> I just verified for-next on QEMU 5.2.0 for virt (RV32,64, nommu) and
>> sifive_u as well.
>> I will give it a try on unleashed tomorrow as well with the above
>> configs enabled.
>>
>> > > I'll take a look again tomorrow night, but if anyone has some time to look
>> > > that'd be great!
>> >
>> > Looks like this breaks on QEMU 5.0.0 but works on 5.2.0.
>>
>> I will take a look tomorrow to check the root cause.
>>
>> I guess technically
>> > that means could be considered a regression, but as we don't really have any
>> > scheme for which old versions of QEMU we support it's not absolute.  I'd
>> > usually err on the side of keeping support for older platforms, but in this
>> > case it's probably just not worth the time so I'm going to just ignore it.
>> >
>> > _______________________________________________
>> > linux-riscv mailing list
>> > linux-riscv@lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>>
>>
>> --
>> Regards,
>> Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-12-23  4:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05  0:04 [PATCH v3 0/5] Improve kernel section protections Atish Patra
2020-11-05  0:04 ` [PATCH v3 1/5] RISC-V: Move __start_kernel to .head.text Atish Patra
2020-11-05  0:04 ` [PATCH v3 2/5] RISC-V: Initialize SBI early Atish Patra
2020-11-05  0:04 ` [PATCH v3 3/5] RISC-V: Align the .init.text section Atish Patra
2020-12-16  6:02   ` Palmer Dabbelt
2020-12-17  6:51     ` Palmer Dabbelt
2020-12-17  8:33       ` Atish Patra
2020-12-18  8:19         ` Atish Patra
2020-12-23  4:14           ` Palmer Dabbelt
2020-11-05  0:04 ` [PATCH v3 4/5] RISC-V: Protect all kernel sections including init early Atish Patra
2020-11-05  0:04 ` [PATCH v3 5/5] RISC-V: Move dynamic relocation section under __init Atish Patra
2020-11-24  7:21 ` [PATCH v3 0/5] Improve kernel section protections Greentime Hu
2020-11-26  0:07   ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).