* [PATCH v8 0/2] Map the kernel with correct permissions the first time
@ 2021-06-24 12:00 Alexandre Ghiti
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Alexandre Ghiti @ 2021-06-24 12:00 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel
Cc: Alexandre Ghiti
The kernel permissions are fixed after the kernel page table is created:
avoid that by mapping the kernel 'correctly' the first time.
Patch 1 introduces a new helper to set kernel mapping permissions while
avoiding all the casts when using set_memory_* API.
Patch 2 is the bulk of this work and deals with mapping the kernel with
the right permissions.
Changes in v8:
* Move set_kernel_memory inline function into set_memory.h header as suggested
by Jisheng
* Make set_kernel_memory arguments name consistent
Changes in v7:
* Split long lines and reintroduce parameters names of set_kernel_memory
callback, as suggested by Christoph
* Make set_kernel_memory __always_inline as suggested by Christoph
* Change 64b spelling into 64-bit, as suggested by Christoph
Changes in v6:
* load_sz was placed in init section but is now used in kernel address
conversions macros, so remove this attribute.
Changes in v5:
* Remove non-relevant commits to this patchset that raised issues
* Make load_sz non-static as it is used in kernel address conversions
macros
* Rebased on top for-next
Changes in v4:
* Add patch 1 as noted by Jisheng
* Changes patch 2 title as suggested by Anup
* Add Reviewed-by from Anup
Changes in v3:
* Add a patch that factorizes kernel address conversions
* Add a helper called set_kernel_memory in its own patch, as suggested by
Christoph
* Prefer IS_ENABLED over #ifdef, as suggested by Christoph
* Split overly long lines, as suggested by Christoph
* Simplify kernel mapping by mapping ALL text as readonly and taking advantage
of already present code that enables write for init text before
free_initmem_default.
Changes in v2:
* Rebased on top of for-next (and "riscv: mm: fix build errors caused by
mk_pmd()")
* Get rid of protect_kernel_linear_mapping_text_rodata as suggested by
Jisheng
* Improve code in general compared to previous RFC
Alexandre Ghiti (2):
riscv: Introduce set_kernel_memory helper
riscv: Map the kernel with correct permissions the first time
arch/riscv/include/asm/page.h | 13 +++-
arch/riscv/include/asm/sections.h | 17 +++++
arch/riscv/include/asm/set_memory.h | 24 ++++--
arch/riscv/kernel/setup.c | 12 +--
arch/riscv/mm/init.c | 112 ++++++++++++----------------
5 files changed, 97 insertions(+), 81 deletions(-)
--
2.30.2
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper
2021-06-24 12:00 [PATCH v8 0/2] Map the kernel with correct permissions the first time Alexandre Ghiti
@ 2021-06-24 12:00 ` Alexandre Ghiti
2021-06-25 1:13 ` Atish Patra
2021-06-25 13:29 ` Jisheng Zhang
2021-06-24 12:00 ` [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time Alexandre Ghiti
2021-07-01 5:37 ` [PATCH v8 0/2] " Palmer Dabbelt
2 siblings, 2 replies; 9+ messages in thread
From: Alexandre Ghiti @ 2021-06-24 12:00 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel
Cc: Alexandre Ghiti
This helper should be used for setting permissions to the kernel
mapping as it takes pointers as arguments and then avoids explicit cast
to unsigned long needed for the set_memory_* API.
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
arch/riscv/include/asm/set_memory.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
index 9d4d455726d4..96e317dcab13 100644
--- a/arch/riscv/include/asm/set_memory.h
+++ b/arch/riscv/include/asm/set_memory.h
@@ -17,6 +17,16 @@ 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);
+static __always_inline int set_kernel_memory(char *startp, char *endp,
+ int (*set_memory)(unsigned long start,
+ int num_pages))
+{
+ unsigned long start = (unsigned long)startp;
+ unsigned long end = (unsigned long)endp;
+ int num_pages = PAGE_ALIGN(end - start) >> PAGE_SHIFT;
+
+ return set_memory(start, num_pages);
+}
#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; }
@@ -24,6 +34,12 @@ 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; }
+static inline int set_kernel_memory(char *startp, char *endp,
+ int (*set_memory)(unsigned long start,
+ int num_pages))
+{
+ return 0;
+}
#endif
#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
--
2.30.2
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time
2021-06-24 12:00 [PATCH v8 0/2] Map the kernel with correct permissions the first time Alexandre Ghiti
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
@ 2021-06-24 12:00 ` Alexandre Ghiti
2021-06-25 1:17 ` Atish Patra
2021-06-25 13:30 ` Jisheng Zhang
2021-07-01 5:37 ` [PATCH v8 0/2] " Palmer Dabbelt
2 siblings, 2 replies; 9+ messages in thread
From: Alexandre Ghiti @ 2021-06-24 12:00 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel
Cc: Alexandre Ghiti
For 64-bit kernels, we map all the kernel with write and execute
permissions and afterwards remove writability from text and executability
from data.
For 32-bit kernels, the kernel mapping resides in the linear mapping, so we
map all the linear mapping as writable and executable and afterwards we
remove those properties for unused memory and kernel mapping as
described above.
Change this behavior to directly map the kernel with correct permissions
and avoid going through the whole mapping to fix the permissions.
At the same time, this fixes an issue introduced by commit 2bfc6cd81bd1
("riscv: Move kernel mapping outside of linear mapping") as reported
here https://github.com/starfive-tech/linux/issues/17.
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
arch/riscv/include/asm/page.h | 13 +++-
arch/riscv/include/asm/sections.h | 17 +++++
arch/riscv/include/asm/set_memory.h | 8 --
arch/riscv/kernel/setup.c | 12 +--
arch/riscv/mm/init.c | 112 ++++++++++++----------------
5 files changed, 81 insertions(+), 81 deletions(-)
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index a1b888f77d57..1c2010c292b3 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -87,6 +87,7 @@ extern unsigned long va_kernel_pa_offset;
extern unsigned long va_kernel_xip_pa_offset;
#endif
extern unsigned long pfn_base;
+extern uintptr_t load_sz;
#define ARCH_PFN_OFFSET (pfn_base)
#else
#define va_pa_offset 0
@@ -99,6 +100,11 @@ extern unsigned long pfn_base;
extern unsigned long kernel_virt_addr;
#ifdef CONFIG_64BIT
+#define is_kernel_mapping(x) \
+ ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
+#define is_linear_mapping(x) \
+ ((x) >= PAGE_OFFSET && (x) < kernel_virt_addr)
+
#define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + va_pa_offset))
#ifdef CONFIG_XIP_KERNEL
#define kernel_mapping_pa_to_va(y) ({ \
@@ -125,10 +131,15 @@ extern unsigned long kernel_virt_addr;
#endif
#define __va_to_pa_nodebug(x) ({ \
unsigned long _x = x; \
- (_x < kernel_virt_addr) ? \
+ is_linear_mapping(_x) ? \
linear_mapping_va_to_pa(_x) : kernel_mapping_va_to_pa(_x); \
})
#else
+#define is_kernel_mapping(x) \
+ ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
+#define is_linear_mapping(x) \
+ ((x) >= PAGE_OFFSET)
+
#define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) + va_pa_offset))
#define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
#endif
diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
index 8a303fb1ee3b..32336e8a17cb 100644
--- a/arch/riscv/include/asm/sections.h
+++ b/arch/riscv/include/asm/sections.h
@@ -6,6 +6,7 @@
#define __ASM_SECTIONS_H
#include <asm-generic/sections.h>
+#include <linux/mm.h>
extern char _start[];
extern char _start_kernel[];
@@ -13,4 +14,20 @@ extern char __init_data_begin[], __init_data_end[];
extern char __init_text_begin[], __init_text_end[];
extern char __alt_start[], __alt_end[];
+static inline bool is_va_kernel_text(uintptr_t va)
+{
+ uintptr_t start = (uintptr_t)_start;
+ uintptr_t end = (uintptr_t)__init_data_begin;
+
+ return va >= start && va < end;
+}
+
+static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
+{
+ uintptr_t start = (uintptr_t)lm_alias(_start);
+ uintptr_t end = (uintptr_t)lm_alias(__init_data_begin);
+
+ return va >= start && va < end;
+}
+
#endif /* __ASM_SECTIONS_H */
diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
index 96e317dcab13..a2c14d4b3993 100644
--- a/arch/riscv/include/asm/set_memory.h
+++ b/arch/riscv/include/asm/set_memory.h
@@ -16,7 +16,6 @@ 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);
static __always_inline int set_kernel_memory(char *startp, char *endp,
int (*set_memory)(unsigned long start,
int num_pages))
@@ -32,7 +31,6 @@ 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; }
static inline int set_kernel_memory(char *startp, char *endp,
int (*set_memory)(unsigned long start,
@@ -42,12 +40,6 @@ static inline int set_kernel_memory(char *startp, char *endp,
}
#endif
-#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
-void __init protect_kernel_linear_mapping_text_rodata(void);
-#else
-static inline void protect_kernel_linear_mapping_text_rodata(void) {}
-#endif
-
int set_direct_map_invalid_noflush(struct page *page);
int set_direct_map_default_noflush(struct page *page);
bool kernel_page_present(struct page *page);
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 5c6d2a1fdbc7..b0c6f372a9ec 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -289,11 +289,6 @@ void __init setup_arch(char **cmdline_p)
init_resources();
sbi_init();
- if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) {
- protect_kernel_text_data();
- protect_kernel_linear_mapping_text_rodata();
- }
-
#ifdef CONFIG_KASAN
kasan_init();
#endif
@@ -328,11 +323,10 @@ subsys_initcall(topology_init);
void free_initmem(void)
{
- unsigned long init_begin = (unsigned long)__init_begin;
- unsigned long init_end = (unsigned long)__init_end;
-
if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
- set_memory_rw_nx(init_begin, (init_end - init_begin) >> PAGE_SHIFT);
+ set_kernel_memory(lm_alias(__init_begin), lm_alias(__init_end),
+ IS_ENABLED(CONFIG_64BIT) ?
+ set_memory_rw : set_memory_rw_nx);
free_initmem_default(POISON_FREE_INITMEM);
}
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b3fd235ddbb6..12f956b3a674 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -455,6 +455,43 @@ asmlinkage void __init __copy_data(void)
}
#endif
+#ifdef CONFIG_STRICT_KERNEL_RWX
+static __init pgprot_t pgprot_from_va(uintptr_t va)
+{
+ if (is_va_kernel_text(va))
+ return PAGE_KERNEL_READ_EXEC;
+
+ /*
+ * In 64-bit kernel, the kernel mapping is outside the linear mapping so
+ * we must protect its linear mapping alias from being executed and
+ * written.
+ * And rodata section is marked readonly in mark_rodata_ro.
+ */
+ if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
+ return PAGE_KERNEL_READ;
+
+ return PAGE_KERNEL;
+}
+
+void mark_rodata_ro(void)
+{
+ set_kernel_memory(__start_rodata, _data, set_memory_ro);
+ if (IS_ENABLED(CONFIG_64BIT))
+ set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
+ set_memory_ro);
+
+ debug_checkwx();
+}
+#else
+static __init pgprot_t pgprot_from_va(uintptr_t va)
+{
+ if (IS_ENABLED(CONFIG_64BIT) && !is_kernel_mapping(va))
+ return PAGE_KERNEL;
+
+ return PAGE_KERNEL_EXEC;
+}
+#endif /* CONFIG_STRICT_KERNEL_RWX */
+
/*
* setup_vm() is called from head.S with MMU-off.
*
@@ -474,7 +511,7 @@ asmlinkage void __init __copy_data(void)
#endif
static uintptr_t load_pa __initdata;
-static uintptr_t load_sz __initdata;
+uintptr_t load_sz;
#ifdef CONFIG_XIP_KERNEL
#define load_pa (*((uintptr_t *)XIP_FIXUP(&load_pa)))
#define load_sz (*((uintptr_t *)XIP_FIXUP(&load_sz)))
@@ -486,7 +523,8 @@ static uintptr_t xiprom_sz __initdata;
#define xiprom_sz (*((uintptr_t *)XIP_FIXUP(&xiprom_sz)))
#define xiprom (*((uintptr_t *)XIP_FIXUP(&xiprom)))
-static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
+static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
+ __always_unused bool early)
{
uintptr_t va, end_va;
@@ -505,7 +543,8 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
map_size, PAGE_KERNEL);
}
#else
-static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
+static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
+ bool early)
{
uintptr_t va, end_va;
@@ -513,7 +552,9 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
for (va = kernel_virt_addr; va < end_va; va += map_size)
create_pgd_mapping(pgdir, va,
load_pa + (va - kernel_virt_addr),
- map_size, PAGE_KERNEL_EXEC);
+ map_size,
+ early ?
+ PAGE_KERNEL_EXEC : pgprot_from_va(va));
}
#endif
@@ -590,7 +631,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
* us to reach paging_init(). We map all memory banks later
* in setup_vm_final() below.
*/
- create_kernel_page_table(early_pg_dir, map_size);
+ create_kernel_page_table(early_pg_dir, map_size, true);
#ifndef __PAGETABLE_PMD_FOLDED
/* Setup early PMD for DTB */
@@ -666,22 +707,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
#endif
}
-#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
-void __init protect_kernel_linear_mapping_text_rodata(void)
-{
- unsigned long text_start = (unsigned long)lm_alias(_start);
- unsigned long init_text_start = (unsigned long)lm_alias(__init_text_begin);
- unsigned long rodata_start = (unsigned long)lm_alias(__start_rodata);
- unsigned long data_start = (unsigned long)lm_alias(_data);
-
- set_memory_ro(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
- set_memory_nx(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
-
- set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
- set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
-}
-#endif
-
static void __init setup_vm_final(void)
{
uintptr_t va, map_size;
@@ -714,21 +739,15 @@ static void __init setup_vm_final(void)
map_size = best_map_size(start, end - start);
for (pa = start; pa < end; pa += map_size) {
va = (uintptr_t)__va(pa);
- create_pgd_mapping(swapper_pg_dir, va, pa,
- map_size,
-#ifdef CONFIG_64BIT
- PAGE_KERNEL
-#else
- PAGE_KERNEL_EXEC
-#endif
- );
+ create_pgd_mapping(swapper_pg_dir, va, pa, map_size,
+ pgprot_from_va(va));
}
}
#ifdef CONFIG_64BIT
/* Map the kernel */
- create_kernel_page_table(swapper_pg_dir, PMD_SIZE);
+ create_kernel_page_table(swapper_pg_dir, PMD_SIZE, false);
#endif
/* Clear fixmap PTE and PMD mappings */
@@ -759,39 +778,6 @@ static inline void setup_vm_final(void)
}
#endif /* CONFIG_MMU */
-#ifdef CONFIG_STRICT_KERNEL_RWX
-void __init protect_kernel_text_data(void)
-{
- 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;
-#if defined(CONFIG_64BIT) && defined(CONFIG_MMU)
- unsigned long end_va = kernel_virt_addr + load_sz;
-#else
- unsigned long end_va = (unsigned long)(__va(PFN_PHYS(max_low_pfn)));
-#endif
-
- 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, (end_va - 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();
-}
-#endif
-
#ifdef CONFIG_KEXEC_CORE
/*
* reserve_crashkernel() - reserves memory for crash kernel
--
2.30.2
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
@ 2021-06-25 1:13 ` Atish Patra
2021-06-25 13:29 ` Jisheng Zhang
1 sibling, 0 replies; 9+ messages in thread
From: Atish Patra @ 2021-06-25 1:13 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel@vger.kernel.org List
On Thu, Jun 24, 2021 at 5:02 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> This helper should be used for setting permissions to the kernel
> mapping as it takes pointers as arguments and then avoids explicit cast
> to unsigned long needed for the set_memory_* API.
>
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
> arch/riscv/include/asm/set_memory.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
> index 9d4d455726d4..96e317dcab13 100644
> --- a/arch/riscv/include/asm/set_memory.h
> +++ b/arch/riscv/include/asm/set_memory.h
> @@ -17,6 +17,16 @@ 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);
> +static __always_inline int set_kernel_memory(char *startp, char *endp,
> + int (*set_memory)(unsigned long start,
> + int num_pages))
> +{
> + unsigned long start = (unsigned long)startp;
> + unsigned long end = (unsigned long)endp;
> + int num_pages = PAGE_ALIGN(end - start) >> PAGE_SHIFT;
> +
> + return set_memory(start, num_pages);
> +}
> #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; }
> @@ -24,6 +34,12 @@ 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; }
> +static inline int set_kernel_memory(char *startp, char *endp,
> + int (*set_memory)(unsigned long start,
> + int num_pages))
> +{
> + return 0;
> +}
> #endif
>
> #if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
> --
> 2.30.2
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Reviewed-by: Atish Patra <atish.patra@wdc.com>
--
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time
2021-06-24 12:00 ` [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time Alexandre Ghiti
@ 2021-06-25 1:17 ` Atish Patra
2021-06-25 13:30 ` Jisheng Zhang
1 sibling, 0 replies; 9+ messages in thread
From: Atish Patra @ 2021-06-25 1:17 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel@vger.kernel.org List
On Thu, Jun 24, 2021 at 5:03 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> For 64-bit kernels, we map all the kernel with write and execute
> permissions and afterwards remove writability from text and executability
> from data.
>
> For 32-bit kernels, the kernel mapping resides in the linear mapping, so we
> map all the linear mapping as writable and executable and afterwards we
> remove those properties for unused memory and kernel mapping as
> described above.
>
> Change this behavior to directly map the kernel with correct permissions
> and avoid going through the whole mapping to fix the permissions.
>
> At the same time, this fixes an issue introduced by commit 2bfc6cd81bd1
> ("riscv: Move kernel mapping outside of linear mapping") as reported
> here https://github.com/starfive-tech/linux/issues/17.
>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
> arch/riscv/include/asm/page.h | 13 +++-
> arch/riscv/include/asm/sections.h | 17 +++++
> arch/riscv/include/asm/set_memory.h | 8 --
> arch/riscv/kernel/setup.c | 12 +--
> arch/riscv/mm/init.c | 112 ++++++++++++----------------
> 5 files changed, 81 insertions(+), 81 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index a1b888f77d57..1c2010c292b3 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -87,6 +87,7 @@ extern unsigned long va_kernel_pa_offset;
> extern unsigned long va_kernel_xip_pa_offset;
> #endif
> extern unsigned long pfn_base;
> +extern uintptr_t load_sz;
> #define ARCH_PFN_OFFSET (pfn_base)
> #else
> #define va_pa_offset 0
> @@ -99,6 +100,11 @@ extern unsigned long pfn_base;
> extern unsigned long kernel_virt_addr;
>
> #ifdef CONFIG_64BIT
> +#define is_kernel_mapping(x) \
> + ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
> +#define is_linear_mapping(x) \
> + ((x) >= PAGE_OFFSET && (x) < kernel_virt_addr)
> +
> #define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + va_pa_offset))
> #ifdef CONFIG_XIP_KERNEL
> #define kernel_mapping_pa_to_va(y) ({ \
> @@ -125,10 +131,15 @@ extern unsigned long kernel_virt_addr;
> #endif
> #define __va_to_pa_nodebug(x) ({ \
> unsigned long _x = x; \
> - (_x < kernel_virt_addr) ? \
> + is_linear_mapping(_x) ? \
> linear_mapping_va_to_pa(_x) : kernel_mapping_va_to_pa(_x); \
> })
> #else
> +#define is_kernel_mapping(x) \
> + ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
> +#define is_linear_mapping(x) \
> + ((x) >= PAGE_OFFSET)
> +
> #define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) + va_pa_offset))
> #define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
> #endif
> diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> index 8a303fb1ee3b..32336e8a17cb 100644
> --- a/arch/riscv/include/asm/sections.h
> +++ b/arch/riscv/include/asm/sections.h
> @@ -6,6 +6,7 @@
> #define __ASM_SECTIONS_H
>
> #include <asm-generic/sections.h>
> +#include <linux/mm.h>
>
> extern char _start[];
> extern char _start_kernel[];
> @@ -13,4 +14,20 @@ extern char __init_data_begin[], __init_data_end[];
> extern char __init_text_begin[], __init_text_end[];
> extern char __alt_start[], __alt_end[];
>
> +static inline bool is_va_kernel_text(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)_start;
> + uintptr_t end = (uintptr_t)__init_data_begin;
> +
> + return va >= start && va < end;
> +}
> +
> +static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)lm_alias(_start);
> + uintptr_t end = (uintptr_t)lm_alias(__init_data_begin);
> +
> + return va >= start && va < end;
> +}
> +
> #endif /* __ASM_SECTIONS_H */
> diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
> index 96e317dcab13..a2c14d4b3993 100644
> --- a/arch/riscv/include/asm/set_memory.h
> +++ b/arch/riscv/include/asm/set_memory.h
> @@ -16,7 +16,6 @@ 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);
> static __always_inline int set_kernel_memory(char *startp, char *endp,
> int (*set_memory)(unsigned long start,
> int num_pages))
> @@ -32,7 +31,6 @@ 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; }
> static inline int set_kernel_memory(char *startp, char *endp,
> int (*set_memory)(unsigned long start,
> @@ -42,12 +40,6 @@ static inline int set_kernel_memory(char *startp, char *endp,
> }
> #endif
>
> -#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
> -void __init protect_kernel_linear_mapping_text_rodata(void);
> -#else
> -static inline void protect_kernel_linear_mapping_text_rodata(void) {}
> -#endif
> -
> int set_direct_map_invalid_noflush(struct page *page);
> int set_direct_map_default_noflush(struct page *page);
> bool kernel_page_present(struct page *page);
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 5c6d2a1fdbc7..b0c6f372a9ec 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -289,11 +289,6 @@ void __init setup_arch(char **cmdline_p)
> init_resources();
> sbi_init();
>
> - if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) {
> - protect_kernel_text_data();
> - protect_kernel_linear_mapping_text_rodata();
> - }
> -
> #ifdef CONFIG_KASAN
> kasan_init();
> #endif
> @@ -328,11 +323,10 @@ subsys_initcall(topology_init);
>
> void free_initmem(void)
> {
> - unsigned long init_begin = (unsigned long)__init_begin;
> - unsigned long init_end = (unsigned long)__init_end;
> -
> if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> - set_memory_rw_nx(init_begin, (init_end - init_begin) >> PAGE_SHIFT);
> + set_kernel_memory(lm_alias(__init_begin), lm_alias(__init_end),
> + IS_ENABLED(CONFIG_64BIT) ?
> + set_memory_rw : set_memory_rw_nx);
>
> free_initmem_default(POISON_FREE_INITMEM);
> }
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b3fd235ddbb6..12f956b3a674 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -455,6 +455,43 @@ asmlinkage void __init __copy_data(void)
> }
> #endif
>
> +#ifdef CONFIG_STRICT_KERNEL_RWX
> +static __init pgprot_t pgprot_from_va(uintptr_t va)
> +{
> + if (is_va_kernel_text(va))
> + return PAGE_KERNEL_READ_EXEC;
> +
> + /*
> + * In 64-bit kernel, the kernel mapping is outside the linear mapping so
> + * we must protect its linear mapping alias from being executed and
> + * written.
> + * And rodata section is marked readonly in mark_rodata_ro.
> + */
> + if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
> + return PAGE_KERNEL_READ;
> +
> + return PAGE_KERNEL;
> +}
> +
> +void mark_rodata_ro(void)
> +{
> + set_kernel_memory(__start_rodata, _data, set_memory_ro);
> + if (IS_ENABLED(CONFIG_64BIT))
> + set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
> + set_memory_ro);
> +
> + debug_checkwx();
> +}
> +#else
> +static __init pgprot_t pgprot_from_va(uintptr_t va)
> +{
> + if (IS_ENABLED(CONFIG_64BIT) && !is_kernel_mapping(va))
> + return PAGE_KERNEL;
> +
> + return PAGE_KERNEL_EXEC;
> +}
> +#endif /* CONFIG_STRICT_KERNEL_RWX */
> +
> /*
> * setup_vm() is called from head.S with MMU-off.
> *
> @@ -474,7 +511,7 @@ asmlinkage void __init __copy_data(void)
> #endif
>
> static uintptr_t load_pa __initdata;
> -static uintptr_t load_sz __initdata;
> +uintptr_t load_sz;
> #ifdef CONFIG_XIP_KERNEL
> #define load_pa (*((uintptr_t *)XIP_FIXUP(&load_pa)))
> #define load_sz (*((uintptr_t *)XIP_FIXUP(&load_sz)))
> @@ -486,7 +523,8 @@ static uintptr_t xiprom_sz __initdata;
> #define xiprom_sz (*((uintptr_t *)XIP_FIXUP(&xiprom_sz)))
> #define xiprom (*((uintptr_t *)XIP_FIXUP(&xiprom)))
>
> -static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
> + __always_unused bool early)
> {
> uintptr_t va, end_va;
>
> @@ -505,7 +543,8 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> map_size, PAGE_KERNEL);
> }
> #else
> -static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
> + bool early)
> {
> uintptr_t va, end_va;
>
> @@ -513,7 +552,9 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> for (va = kernel_virt_addr; va < end_va; va += map_size)
> create_pgd_mapping(pgdir, va,
> load_pa + (va - kernel_virt_addr),
> - map_size, PAGE_KERNEL_EXEC);
> + map_size,
> + early ?
> + PAGE_KERNEL_EXEC : pgprot_from_va(va));
> }
> #endif
>
> @@ -590,7 +631,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> * us to reach paging_init(). We map all memory banks later
> * in setup_vm_final() below.
> */
> - create_kernel_page_table(early_pg_dir, map_size);
> + create_kernel_page_table(early_pg_dir, map_size, true);
>
> #ifndef __PAGETABLE_PMD_FOLDED
> /* Setup early PMD for DTB */
> @@ -666,22 +707,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> #endif
> }
>
> -#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
> -void __init protect_kernel_linear_mapping_text_rodata(void)
> -{
> - unsigned long text_start = (unsigned long)lm_alias(_start);
> - unsigned long init_text_start = (unsigned long)lm_alias(__init_text_begin);
> - unsigned long rodata_start = (unsigned long)lm_alias(__start_rodata);
> - unsigned long data_start = (unsigned long)lm_alias(_data);
> -
> - set_memory_ro(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
> - set_memory_nx(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
> -
> - set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
> - set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
> -}
> -#endif
> -
> static void __init setup_vm_final(void)
> {
> uintptr_t va, map_size;
> @@ -714,21 +739,15 @@ static void __init setup_vm_final(void)
> map_size = best_map_size(start, end - start);
> for (pa = start; pa < end; pa += map_size) {
> va = (uintptr_t)__va(pa);
> - create_pgd_mapping(swapper_pg_dir, va, pa,
> - map_size,
> -#ifdef CONFIG_64BIT
> - PAGE_KERNEL
> -#else
> - PAGE_KERNEL_EXEC
> -#endif
> - );
>
> + create_pgd_mapping(swapper_pg_dir, va, pa, map_size,
> + pgprot_from_va(va));
> }
> }
>
> #ifdef CONFIG_64BIT
> /* Map the kernel */
> - create_kernel_page_table(swapper_pg_dir, PMD_SIZE);
> + create_kernel_page_table(swapper_pg_dir, PMD_SIZE, false);
> #endif
>
> /* Clear fixmap PTE and PMD mappings */
> @@ -759,39 +778,6 @@ static inline void setup_vm_final(void)
> }
> #endif /* CONFIG_MMU */
>
> -#ifdef CONFIG_STRICT_KERNEL_RWX
> -void __init protect_kernel_text_data(void)
> -{
> - 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;
> -#if defined(CONFIG_64BIT) && defined(CONFIG_MMU)
> - unsigned long end_va = kernel_virt_addr + load_sz;
> -#else
> - unsigned long end_va = (unsigned long)(__va(PFN_PHYS(max_low_pfn)));
> -#endif
> -
> - 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, (end_va - 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();
> -}
> -#endif
> -
> #ifdef CONFIG_KEXEC_CORE
> /*
> * reserve_crashkernel() - reserves memory for crash kernel
> --
> 2.30.2
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Reviewed-by: Atish Patra <atish.patra@wdc.com>
--
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
2021-06-25 1:13 ` Atish Patra
@ 2021-06-25 13:29 ` Jisheng Zhang
1 sibling, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-06-25 13:29 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel
On Thu, 24 Jun 2021 14:00:40 +0200
Alexandre Ghiti <alex@ghiti.fr> wrote:
> This helper should be used for setting permissions to the kernel
> mapping as it takes pointers as arguments and then avoids explicit cast
> to unsigned long needed for the set_memory_* API.
>
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/set_memory.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
> index 9d4d455726d4..96e317dcab13 100644
> --- a/arch/riscv/include/asm/set_memory.h
> +++ b/arch/riscv/include/asm/set_memory.h
> @@ -17,6 +17,16 @@ 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);
> +static __always_inline int set_kernel_memory(char *startp, char *endp,
> + int (*set_memory)(unsigned long start,
> + int num_pages))
> +{
> + unsigned long start = (unsigned long)startp;
> + unsigned long end = (unsigned long)endp;
> + int num_pages = PAGE_ALIGN(end - start) >> PAGE_SHIFT;
> +
> + return set_memory(start, num_pages);
> +}
> #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; }
> @@ -24,6 +34,12 @@ 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; }
> +static inline int set_kernel_memory(char *startp, char *endp,
> + int (*set_memory)(unsigned long start,
> + int num_pages))
> +{
> + return 0;
> +}
> #endif
>
> #if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time
2021-06-24 12:00 ` [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time Alexandre Ghiti
2021-06-25 1:17 ` Atish Patra
@ 2021-06-25 13:30 ` Jisheng Zhang
1 sibling, 0 replies; 9+ messages in thread
From: Jisheng Zhang @ 2021-06-25 13:30 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Jisheng Zhang,
Christoph Hellwig, Zong Li, Anup Patel, linux-riscv,
linux-kernel
On Thu, 24 Jun 2021 14:00:41 +0200
Alexandre Ghiti <alex@ghiti.fr> wrote:
> For 64-bit kernels, we map all the kernel with write and execute
> permissions and afterwards remove writability from text and executability
> from data.
>
> For 32-bit kernels, the kernel mapping resides in the linear mapping, so we
> map all the linear mapping as writable and executable and afterwards we
> remove those properties for unused memory and kernel mapping as
> described above.
>
> Change this behavior to directly map the kernel with correct permissions
> and avoid going through the whole mapping to fix the permissions.
>
> At the same time, this fixes an issue introduced by commit 2bfc6cd81bd1
> ("riscv: Move kernel mapping outside of linear mapping") as reported
> here https://github.com/starfive-tech/linux/issues/17.
>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Reviewed-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/page.h | 13 +++-
> arch/riscv/include/asm/sections.h | 17 +++++
> arch/riscv/include/asm/set_memory.h | 8 --
> arch/riscv/kernel/setup.c | 12 +--
> arch/riscv/mm/init.c | 112 ++++++++++++----------------
> 5 files changed, 81 insertions(+), 81 deletions(-)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index a1b888f77d57..1c2010c292b3 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -87,6 +87,7 @@ extern unsigned long va_kernel_pa_offset;
> extern unsigned long va_kernel_xip_pa_offset;
> #endif
> extern unsigned long pfn_base;
> +extern uintptr_t load_sz;
> #define ARCH_PFN_OFFSET (pfn_base)
> #else
> #define va_pa_offset 0
> @@ -99,6 +100,11 @@ extern unsigned long pfn_base;
> extern unsigned long kernel_virt_addr;
>
> #ifdef CONFIG_64BIT
> +#define is_kernel_mapping(x) \
> + ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
> +#define is_linear_mapping(x) \
> + ((x) >= PAGE_OFFSET && (x) < kernel_virt_addr)
> +
> #define linear_mapping_pa_to_va(x) ((void *)((unsigned long)(x) + va_pa_offset))
> #ifdef CONFIG_XIP_KERNEL
> #define kernel_mapping_pa_to_va(y) ({ \
> @@ -125,10 +131,15 @@ extern unsigned long kernel_virt_addr;
> #endif
> #define __va_to_pa_nodebug(x) ({ \
> unsigned long _x = x; \
> - (_x < kernel_virt_addr) ? \
> + is_linear_mapping(_x) ? \
> linear_mapping_va_to_pa(_x) : kernel_mapping_va_to_pa(_x); \
> })
> #else
> +#define is_kernel_mapping(x) \
> + ((x) >= kernel_virt_addr && (x) < (kernel_virt_addr + load_sz))
> +#define is_linear_mapping(x) \
> + ((x) >= PAGE_OFFSET)
> +
> #define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) + va_pa_offset))
> #define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
> #endif
> diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> index 8a303fb1ee3b..32336e8a17cb 100644
> --- a/arch/riscv/include/asm/sections.h
> +++ b/arch/riscv/include/asm/sections.h
> @@ -6,6 +6,7 @@
> #define __ASM_SECTIONS_H
>
> #include <asm-generic/sections.h>
> +#include <linux/mm.h>
>
> extern char _start[];
> extern char _start_kernel[];
> @@ -13,4 +14,20 @@ extern char __init_data_begin[], __init_data_end[];
> extern char __init_text_begin[], __init_text_end[];
> extern char __alt_start[], __alt_end[];
>
> +static inline bool is_va_kernel_text(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)_start;
> + uintptr_t end = (uintptr_t)__init_data_begin;
> +
> + return va >= start && va < end;
> +}
> +
> +static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)lm_alias(_start);
> + uintptr_t end = (uintptr_t)lm_alias(__init_data_begin);
> +
> + return va >= start && va < end;
> +}
> +
> #endif /* __ASM_SECTIONS_H */
> diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/asm/set_memory.h
> index 96e317dcab13..a2c14d4b3993 100644
> --- a/arch/riscv/include/asm/set_memory.h
> +++ b/arch/riscv/include/asm/set_memory.h
> @@ -16,7 +16,6 @@ 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);
> static __always_inline int set_kernel_memory(char *startp, char *endp,
> int (*set_memory)(unsigned long start,
> int num_pages))
> @@ -32,7 +31,6 @@ 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; }
> static inline int set_kernel_memory(char *startp, char *endp,
> int (*set_memory)(unsigned long start,
> @@ -42,12 +40,6 @@ static inline int set_kernel_memory(char *startp, char *endp,
> }
> #endif
>
> -#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
> -void __init protect_kernel_linear_mapping_text_rodata(void);
> -#else
> -static inline void protect_kernel_linear_mapping_text_rodata(void) {}
> -#endif
> -
> int set_direct_map_invalid_noflush(struct page *page);
> int set_direct_map_default_noflush(struct page *page);
> bool kernel_page_present(struct page *page);
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 5c6d2a1fdbc7..b0c6f372a9ec 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -289,11 +289,6 @@ void __init setup_arch(char **cmdline_p)
> init_resources();
> sbi_init();
>
> - if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) {
> - protect_kernel_text_data();
> - protect_kernel_linear_mapping_text_rodata();
> - }
> -
> #ifdef CONFIG_KASAN
> kasan_init();
> #endif
> @@ -328,11 +323,10 @@ subsys_initcall(topology_init);
>
> void free_initmem(void)
> {
> - unsigned long init_begin = (unsigned long)__init_begin;
> - unsigned long init_end = (unsigned long)__init_end;
> -
> if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> - set_memory_rw_nx(init_begin, (init_end - init_begin) >> PAGE_SHIFT);
> + set_kernel_memory(lm_alias(__init_begin), lm_alias(__init_end),
> + IS_ENABLED(CONFIG_64BIT) ?
> + set_memory_rw : set_memory_rw_nx);
>
> free_initmem_default(POISON_FREE_INITMEM);
> }
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b3fd235ddbb6..12f956b3a674 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -455,6 +455,43 @@ asmlinkage void __init __copy_data(void)
> }
> #endif
>
> +#ifdef CONFIG_STRICT_KERNEL_RWX
> +static __init pgprot_t pgprot_from_va(uintptr_t va)
> +{
> + if (is_va_kernel_text(va))
> + return PAGE_KERNEL_READ_EXEC;
> +
> + /*
> + * In 64-bit kernel, the kernel mapping is outside the linear mapping so
> + * we must protect its linear mapping alias from being executed and
> + * written.
> + * And rodata section is marked readonly in mark_rodata_ro.
> + */
> + if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
> + return PAGE_KERNEL_READ;
> +
> + return PAGE_KERNEL;
> +}
> +
> +void mark_rodata_ro(void)
> +{
> + set_kernel_memory(__start_rodata, _data, set_memory_ro);
> + if (IS_ENABLED(CONFIG_64BIT))
> + set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
> + set_memory_ro);
> +
> + debug_checkwx();
> +}
> +#else
> +static __init pgprot_t pgprot_from_va(uintptr_t va)
> +{
> + if (IS_ENABLED(CONFIG_64BIT) && !is_kernel_mapping(va))
> + return PAGE_KERNEL;
> +
> + return PAGE_KERNEL_EXEC;
> +}
> +#endif /* CONFIG_STRICT_KERNEL_RWX */
> +
> /*
> * setup_vm() is called from head.S with MMU-off.
> *
> @@ -474,7 +511,7 @@ asmlinkage void __init __copy_data(void)
> #endif
>
> static uintptr_t load_pa __initdata;
> -static uintptr_t load_sz __initdata;
> +uintptr_t load_sz;
> #ifdef CONFIG_XIP_KERNEL
> #define load_pa (*((uintptr_t *)XIP_FIXUP(&load_pa)))
> #define load_sz (*((uintptr_t *)XIP_FIXUP(&load_sz)))
> @@ -486,7 +523,8 @@ static uintptr_t xiprom_sz __initdata;
> #define xiprom_sz (*((uintptr_t *)XIP_FIXUP(&xiprom_sz)))
> #define xiprom (*((uintptr_t *)XIP_FIXUP(&xiprom)))
>
> -static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
> + __always_unused bool early)
> {
> uintptr_t va, end_va;
>
> @@ -505,7 +543,8 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> map_size, PAGE_KERNEL);
> }
> #else
> -static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size,
> + bool early)
> {
> uintptr_t va, end_va;
>
> @@ -513,7 +552,9 @@ static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> for (va = kernel_virt_addr; va < end_va; va += map_size)
> create_pgd_mapping(pgdir, va,
> load_pa + (va - kernel_virt_addr),
> - map_size, PAGE_KERNEL_EXEC);
> + map_size,
> + early ?
> + PAGE_KERNEL_EXEC : pgprot_from_va(va));
> }
> #endif
>
> @@ -590,7 +631,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> * us to reach paging_init(). We map all memory banks later
> * in setup_vm_final() below.
> */
> - create_kernel_page_table(early_pg_dir, map_size);
> + create_kernel_page_table(early_pg_dir, map_size, true);
>
> #ifndef __PAGETABLE_PMD_FOLDED
> /* Setup early PMD for DTB */
> @@ -666,22 +707,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> #endif
> }
>
> -#if defined(CONFIG_64BIT) && defined(CONFIG_STRICT_KERNEL_RWX)
> -void __init protect_kernel_linear_mapping_text_rodata(void)
> -{
> - unsigned long text_start = (unsigned long)lm_alias(_start);
> - unsigned long init_text_start = (unsigned long)lm_alias(__init_text_begin);
> - unsigned long rodata_start = (unsigned long)lm_alias(__start_rodata);
> - unsigned long data_start = (unsigned long)lm_alias(_data);
> -
> - set_memory_ro(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
> - set_memory_nx(text_start, (init_text_start - text_start) >> PAGE_SHIFT);
> -
> - set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
> - set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT);
> -}
> -#endif
> -
> static void __init setup_vm_final(void)
> {
> uintptr_t va, map_size;
> @@ -714,21 +739,15 @@ static void __init setup_vm_final(void)
> map_size = best_map_size(start, end - start);
> for (pa = start; pa < end; pa += map_size) {
> va = (uintptr_t)__va(pa);
> - create_pgd_mapping(swapper_pg_dir, va, pa,
> - map_size,
> -#ifdef CONFIG_64BIT
> - PAGE_KERNEL
> -#else
> - PAGE_KERNEL_EXEC
> -#endif
> - );
>
> + create_pgd_mapping(swapper_pg_dir, va, pa, map_size,
> + pgprot_from_va(va));
> }
> }
>
> #ifdef CONFIG_64BIT
> /* Map the kernel */
> - create_kernel_page_table(swapper_pg_dir, PMD_SIZE);
> + create_kernel_page_table(swapper_pg_dir, PMD_SIZE, false);
> #endif
>
> /* Clear fixmap PTE and PMD mappings */
> @@ -759,39 +778,6 @@ static inline void setup_vm_final(void)
> }
> #endif /* CONFIG_MMU */
>
> -#ifdef CONFIG_STRICT_KERNEL_RWX
> -void __init protect_kernel_text_data(void)
> -{
> - 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;
> -#if defined(CONFIG_64BIT) && defined(CONFIG_MMU)
> - unsigned long end_va = kernel_virt_addr + load_sz;
> -#else
> - unsigned long end_va = (unsigned long)(__va(PFN_PHYS(max_low_pfn)));
> -#endif
> -
> - 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, (end_va - 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();
> -}
> -#endif
> -
> #ifdef CONFIG_KEXEC_CORE
> /*
> * reserve_crashkernel() - reserves memory for crash kernel
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 0/2] Map the kernel with correct permissions the first time
2021-06-24 12:00 [PATCH v8 0/2] Map the kernel with correct permissions the first time Alexandre Ghiti
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
2021-06-24 12:00 ` [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time Alexandre Ghiti
@ 2021-07-01 5:37 ` Palmer Dabbelt
2021-07-01 19:31 ` Alex Ghiti
2 siblings, 1 reply; 9+ messages in thread
From: Palmer Dabbelt @ 2021-07-01 5:37 UTC (permalink / raw)
To: alex
Cc: Paul Walmsley, aou, jszhang, Christoph Hellwig, zong.li, anup,
linux-riscv, linux-kernel, alex
On Thu, 24 Jun 2021 05:00:39 PDT (-0700), alex@ghiti.fr wrote:
> The kernel permissions are fixed after the kernel page table is created:
> avoid that by mapping the kernel 'correctly' the first time.
>
> Patch 1 introduces a new helper to set kernel mapping permissions while
> avoiding all the casts when using set_memory_* API.
>
> Patch 2 is the bulk of this work and deals with mapping the kernel with
> the right permissions.
>
> Changes in v8:
> * Move set_kernel_memory inline function into set_memory.h header as suggested
> by Jisheng
> * Make set_kernel_memory arguments name consistent
>
> Changes in v7:
> * Split long lines and reintroduce parameters names of set_kernel_memory
> callback, as suggested by Christoph
> * Make set_kernel_memory __always_inline as suggested by Christoph
> * Change 64b spelling into 64-bit, as suggested by Christoph
>
> Changes in v6:
> * load_sz was placed in init section but is now used in kernel address
> conversions macros, so remove this attribute.
>
> Changes in v5:
> * Remove non-relevant commits to this patchset that raised issues
> * Make load_sz non-static as it is used in kernel address conversions
> macros
> * Rebased on top for-next
>
> Changes in v4:
> * Add patch 1 as noted by Jisheng
> * Changes patch 2 title as suggested by Anup
> * Add Reviewed-by from Anup
>
> Changes in v3:
> * Add a patch that factorizes kernel address conversions
> * Add a helper called set_kernel_memory in its own patch, as suggested by
> Christoph
> * Prefer IS_ENABLED over #ifdef, as suggested by Christoph
> * Split overly long lines, as suggested by Christoph
> * Simplify kernel mapping by mapping ALL text as readonly and taking advantage
> of already present code that enables write for init text before
> free_initmem_default.
>
> Changes in v2:
> * Rebased on top of for-next (and "riscv: mm: fix build errors caused by
> mk_pmd()")
> * Get rid of protect_kernel_linear_mapping_text_rodata as suggested by
> Jisheng
> * Improve code in general compared to previous RFC
>
> Alexandre Ghiti (2):
> riscv: Introduce set_kernel_memory helper
> riscv: Map the kernel with correct permissions the first time
>
> arch/riscv/include/asm/page.h | 13 +++-
> arch/riscv/include/asm/sections.h | 17 +++++
> arch/riscv/include/asm/set_memory.h | 24 ++++--
> arch/riscv/kernel/setup.c | 12 +--
> arch/riscv/mm/init.c | 112 ++++++++++++----------------
> 5 files changed, 97 insertions(+), 81 deletions(-)
I had what looks to be two earlier versions of these patches on
for-next. I've fixed that up, but there were some merge conflicts.
Let me know if there were any issues, but it's really getting too late
in the cycle to be rebasing so I'd prefer just a fixup.
Thanks!
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 0/2] Map the kernel with correct permissions the first time
2021-07-01 5:37 ` [PATCH v8 0/2] " Palmer Dabbelt
@ 2021-07-01 19:31 ` Alex Ghiti
0 siblings, 0 replies; 9+ messages in thread
From: Alex Ghiti @ 2021-07-01 19:31 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Paul Walmsley, aou, jszhang, Christoph Hellwig, zong.li, anup,
linux-riscv, linux-kernel
Le 1/07/2021 à 07:37, Palmer Dabbelt a écrit :
> On Thu, 24 Jun 2021 05:00:39 PDT (-0700), alex@ghiti.fr wrote:
>> The kernel permissions are fixed after the kernel page table is created:
>> avoid that by mapping the kernel 'correctly' the first time.
>>
>> Patch 1 introduces a new helper to set kernel mapping permissions while
>> avoiding all the casts when using set_memory_* API.
>>
>> Patch 2 is the bulk of this work and deals with mapping the kernel with
>> the right permissions.
>>
>> Changes in v8:
>> * Move set_kernel_memory inline function into set_memory.h header as
>> suggested
>> by Jisheng
>> * Make set_kernel_memory arguments name consistent
>>
>> Changes in v7:
>> * Split long lines and reintroduce parameters names of set_kernel_memory
>> callback, as suggested by Christoph
>> * Make set_kernel_memory __always_inline as suggested by Christoph
>> * Change 64b spelling into 64-bit, as suggested by Christoph
>>
>> Changes in v6:
>> * load_sz was placed in init section but is now used in kernel address
>> conversions macros, so remove this attribute.
>>
>> Changes in v5:
>> * Remove non-relevant commits to this patchset that raised issues
>> * Make load_sz non-static as it is used in kernel address conversions
>> macros
>> * Rebased on top for-next
>>
>> Changes in v4:
>> * Add patch 1 as noted by Jisheng
>> * Changes patch 2 title as suggested by Anup
>> * Add Reviewed-by from Anup
>>
>> Changes in v3:
>> * Add a patch that factorizes kernel address conversions
>> * Add a helper called set_kernel_memory in its own patch, as suggested by
>> Christoph
>> * Prefer IS_ENABLED over #ifdef, as suggested by Christoph
>> * Split overly long lines, as suggested by Christoph
>> * Simplify kernel mapping by mapping ALL text as readonly and taking
>> advantage
>> of already present code that enables write for init text before
>> free_initmem_default.
>>
>> Changes in v2:
>> * Rebased on top of for-next (and "riscv: mm: fix build errors caused by
>> mk_pmd()")
>> * Get rid of protect_kernel_linear_mapping_text_rodata as suggested by
>> Jisheng
>> * Improve code in general compared to previous RFC
>>
>> Alexandre Ghiti (2):
>> riscv: Introduce set_kernel_memory helper
>> riscv: Map the kernel with correct permissions the first time
>>
>> arch/riscv/include/asm/page.h | 13 +++-
>> arch/riscv/include/asm/sections.h | 17 +++++
>> arch/riscv/include/asm/set_memory.h | 24 ++++--
>> arch/riscv/kernel/setup.c | 12 +--
>> arch/riscv/mm/init.c | 112 ++++++++++++----------------
>> 5 files changed, 97 insertions(+), 81 deletions(-)
>
> I had what looks to be two earlier versions of these patches on
> for-next. I've fixed that up, but there were some merge conflicts. Let
> me know if there were any issues, but it's really getting too late in
> the cycle to be rebasing so I'd prefer just a fixup.
>
The first 2 patches:
riscv: Remove CONFIG_PHYS_RAM_BASE_FIXED
riscv: Simplify xip and !xip kernel address conversion macros
should *not* be merged as they assume the DRAM physical base address is
0x8000_0000 for *all* rv64 chips. I have another patchset coming to
replace those 2 patches.
Alex
> Thanks!
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-01 19:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 12:00 [PATCH v8 0/2] Map the kernel with correct permissions the first time Alexandre Ghiti
2021-06-24 12:00 ` [PATCH v8 1/2] riscv: Introduce set_kernel_memory helper Alexandre Ghiti
2021-06-25 1:13 ` Atish Patra
2021-06-25 13:29 ` Jisheng Zhang
2021-06-24 12:00 ` [PATCH v8 2/2] riscv: Map the kernel with correct permissions the first time Alexandre Ghiti
2021-06-25 1:17 ` Atish Patra
2021-06-25 13:30 ` Jisheng Zhang
2021-07-01 5:37 ` [PATCH v8 0/2] " Palmer Dabbelt
2021-07-01 19:31 ` Alex Ghiti
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).