All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-16  9:00 ` Jisheng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-16  9:00 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

From: Jisheng Zhang <jszhang@kernel.org>

When the kernel mapping was moved the last 2GB of the address space,
(__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
start address, the last set_memory_nx() in protect_kernel_text_data()
will fail, thus the .data section is still mapped as W+X. This results
in below W+X mapping waring at boot. Fix it by passing the correct
.data section page num to the set_memory_nx().

[    0.396516] ------------[ cut here ]------------
[    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
[    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
[    0.398964] Modules linked in:
[    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
[    0.400003] Hardware name: riscv-virtio,qemu (DT)
[    0.400591] epc : note_page+0x244/0x24a
[    0.401368]  ra : note_page+0x244/0x24a
[    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
[    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
[    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
[    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
[    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
[    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
[    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
[    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
[    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
[    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
[    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
[    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
[    0.408052] Call Trace:
[    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
[    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
[    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
[    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
[    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
[    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
[    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
[    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
[    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
[    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
[    0.413141] Checked W+X mappings: failed, 512 W+X pages found

Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/mm/init.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 4faf8bd157ea..4c4c92ce0bb8 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
 	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)));
+#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, (max_low - data_start) >> PAGE_SHIFT);
+	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
 }
 
 void mark_rodata_ro(void)
-- 
2.31.0



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

* [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-16  9:00 ` Jisheng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-16  9:00 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: linux-riscv, linux-kernel

From: Jisheng Zhang <jszhang@kernel.org>

When the kernel mapping was moved the last 2GB of the address space,
(__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
start address, the last set_memory_nx() in protect_kernel_text_data()
will fail, thus the .data section is still mapped as W+X. This results
in below W+X mapping waring at boot. Fix it by passing the correct
.data section page num to the set_memory_nx().

[    0.396516] ------------[ cut here ]------------
[    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
[    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
[    0.398964] Modules linked in:
[    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
[    0.400003] Hardware name: riscv-virtio,qemu (DT)
[    0.400591] epc : note_page+0x244/0x24a
[    0.401368]  ra : note_page+0x244/0x24a
[    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
[    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
[    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
[    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
[    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
[    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
[    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
[    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
[    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
[    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
[    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
[    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
[    0.408052] Call Trace:
[    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
[    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
[    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
[    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
[    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
[    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
[    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
[    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
[    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
[    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
[    0.413141] Checked W+X mappings: failed, 512 W+X pages found

Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/mm/init.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 4faf8bd157ea..4c4c92ce0bb8 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
 	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)));
+#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, (max_low - data_start) >> PAGE_SHIFT);
+	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
 }
 
 void mark_rodata_ro(void)
-- 
2.31.0



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
  2021-05-16  9:00 ` Jisheng Zhang
@ 2021-05-18 15:26   ` Alex Ghiti
  -1 siblings, 0 replies; 12+ messages in thread
From: Alex Ghiti @ 2021-05-18 15:26 UTC (permalink / raw)
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: linux-riscv, linux-kernel

Hi Jisheng,

On 16/05/2021 11:00, Jisheng Zhang wrote:
> From: Jisheng Zhang <jszhang@kernel.org>
> 
> When the kernel mapping was moved the last 2GB of the address space,
> (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> start address, the last set_memory_nx() in protect_kernel_text_data()
> will fail, thus the .data section is still mapped as W+X. This results
> in below W+X mapping waring at boot. Fix it by passing the correct
> .data section page num to the set_memory_nx().
> 
> [    0.396516] ------------[ cut here ]------------
> [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> [    0.398964] Modules linked in:
> [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> [    0.400591] epc : note_page+0x244/0x24a
> [    0.401368]  ra : note_page+0x244/0x24a
> [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.408052] Call Trace:
> [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> 
> Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>   arch/riscv/mm/init.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4faf8bd157ea..4c4c92ce0bb8 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>   	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)));
> +#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, (max_low - data_start) >> PAGE_SHIFT);
> +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>   }
>   
>   void mark_rodata_ro(void)
> 

Thank you for taking the time to fix this, I had read a report here 
https://github.com/starfive-tech/linux/issues/17 but had no time yet to 
track this down.

Your fix seems good to me, but it intrigued me to see that for 32b 
kernels, the whole linear mapping is mapped as executable and then here, 
we remove this attribute. So I came up with a patch to map the kernel 
correctly once at first time and avoid fixing this mapping afterwards. I 
added you in cc of this patch, any comment is welcome.

Thanks,

Alex

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-18 15:26   ` Alex Ghiti
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Ghiti @ 2021-05-18 15:26 UTC (permalink / raw)
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: linux-riscv, linux-kernel

Hi Jisheng,

On 16/05/2021 11:00, Jisheng Zhang wrote:
> From: Jisheng Zhang <jszhang@kernel.org>
> 
> When the kernel mapping was moved the last 2GB of the address space,
> (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> start address, the last set_memory_nx() in protect_kernel_text_data()
> will fail, thus the .data section is still mapped as W+X. This results
> in below W+X mapping waring at boot. Fix it by passing the correct
> .data section page num to the set_memory_nx().
> 
> [    0.396516] ------------[ cut here ]------------
> [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> [    0.398964] Modules linked in:
> [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> [    0.400591] epc : note_page+0x244/0x24a
> [    0.401368]  ra : note_page+0x244/0x24a
> [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.408052] Call Trace:
> [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> 
> Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>   arch/riscv/mm/init.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4faf8bd157ea..4c4c92ce0bb8 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>   	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)));
> +#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, (max_low - data_start) >> PAGE_SHIFT);
> +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>   }
>   
>   void mark_rodata_ro(void)
> 

Thank you for taking the time to fix this, I had read a report here 
https://github.com/starfive-tech/linux/issues/17 but had no time yet to 
track this down.

Your fix seems good to me, but it intrigued me to see that for 32b 
kernels, the whole linear mapping is mapped as executable and then here, 
we remove this attribute. So I came up with a patch to map the kernel 
correctly once at first time and avoid fixing this mapping afterwards. I 
added you in cc of this patch, any comment is welcome.

Thanks,

Alex

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
  2021-05-18 15:26   ` Alex Ghiti
@ 2021-05-19 15:23     ` Jisheng Zhang
  -1 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-19 15:23 UTC (permalink / raw)
  To: Alex Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel


On Tue, 18 May 2021 17:26:53 +0200
Alex Ghiti <alex@ghiti.fr> wrote:

> Hi Jisheng,

Hi Alex,

> 
> On 16/05/2021 11:00, Jisheng Zhang wrote:
> > From: Jisheng Zhang <jszhang@kernel.org>
> > 
> > When the kernel mapping was moved the last 2GB of the address space,
> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> > start address, the last set_memory_nx() in protect_kernel_text_data()
> > will fail, thus the .data section is still mapped as W+X. This results
> > in below W+X mapping waring at boot. Fix it by passing the correct
> > .data section page num to the set_memory_nx().
> > 
> > [    0.396516] ------------[ cut here ]------------
> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> > [    0.398964] Modules linked in:
> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> > [    0.400591] epc : note_page+0x244/0x24a
> > [    0.401368]  ra : note_page+0x244/0x24a
> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.408052] Call Trace:
> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> > 
> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >   arch/riscv/mm/init.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4faf8bd157ea..4c4c92ce0bb8 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
> >   	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)));
> > +#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, (max_low - data_start) >> PAGE_SHIFT);
> > +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
> >   }
> >   
> >   void mark_rodata_ro(void)
> >   
> 
> Thank you for taking the time to fix this, I had read a report here 
> https://github.com/starfive-tech/linux/issues/17 but had no time yet to 

I didn't know this github repo, in fact I don't have a beaglev board ;)
From the log, this is the same issue as I saw on Qemu.

> track this down.
> 
> Your fix seems good to me, but it intrigued me to see that for 32b 
> kernels, the whole linear mapping is mapped as executable and then here, 
> we remove this attribute. So I came up with a patch to map the kernel 
> correctly once at first time and avoid fixing this mapping afterwards. I 

Your solution looks better.

> added you in cc of this patch, any comment is welcome.
> 

I'm reading your patch. Will comment it soon.

Thanks



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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-19 15:23     ` Jisheng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-19 15:23 UTC (permalink / raw)
  To: Alex Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv, linux-kernel


On Tue, 18 May 2021 17:26:53 +0200
Alex Ghiti <alex@ghiti.fr> wrote:

> Hi Jisheng,

Hi Alex,

> 
> On 16/05/2021 11:00, Jisheng Zhang wrote:
> > From: Jisheng Zhang <jszhang@kernel.org>
> > 
> > When the kernel mapping was moved the last 2GB of the address space,
> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> > start address, the last set_memory_nx() in protect_kernel_text_data()
> > will fail, thus the .data section is still mapped as W+X. This results
> > in below W+X mapping waring at boot. Fix it by passing the correct
> > .data section page num to the set_memory_nx().
> > 
> > [    0.396516] ------------[ cut here ]------------
> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> > [    0.398964] Modules linked in:
> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> > [    0.400591] epc : note_page+0x244/0x24a
> > [    0.401368]  ra : note_page+0x244/0x24a
> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.408052] Call Trace:
> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> > 
> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >   arch/riscv/mm/init.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4faf8bd157ea..4c4c92ce0bb8 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
> >   	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)));
> > +#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, (max_low - data_start) >> PAGE_SHIFT);
> > +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
> >   }
> >   
> >   void mark_rodata_ro(void)
> >   
> 
> Thank you for taking the time to fix this, I had read a report here 
> https://github.com/starfive-tech/linux/issues/17 but had no time yet to 

I didn't know this github repo, in fact I don't have a beaglev board ;)
From the log, this is the same issue as I saw on Qemu.

> track this down.
> 
> Your fix seems good to me, but it intrigued me to see that for 32b 
> kernels, the whole linear mapping is mapped as executable and then here, 
> we remove this attribute. So I came up with a patch to map the kernel 
> correctly once at first time and avoid fixing this mapping afterwards. I 

Your solution looks better.

> added you in cc of this patch, any comment is welcome.
> 

I'm reading your patch. Will comment it soon.

Thanks



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
  2021-05-16  9:00 ` Jisheng Zhang
@ 2021-05-20  4:55   ` Drew Fustini
  -1 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2021-05-20  4:55 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	linux-riscv, linux-kernel, Wei Fu, Emil Renner Berthing

On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
> From: Jisheng Zhang <jszhang@kernel.org>
> 
> When the kernel mapping was moved the last 2GB of the address space,
> (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> start address, the last set_memory_nx() in protect_kernel_text_data()
> will fail, thus the .data section is still mapped as W+X. This results
> in below W+X mapping waring at boot. Fix it by passing the correct
> .data section page num to the set_memory_nx().
> 
> [    0.396516] ------------[ cut here ]------------
> [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> [    0.398964] Modules linked in:
> [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> [    0.400591] epc : note_page+0x244/0x24a
> [    0.401368]  ra : note_page+0x244/0x24a
> [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.408052] Call Trace:
> [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> 
> Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/mm/init.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4faf8bd157ea..4c4c92ce0bb8 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>  	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)));
> +#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, (max_low - data_start) >> PAGE_SHIFT);
> +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>  }
>  
>  void mark_rodata_ro(void)
> -- 
> 2.31.0
> 
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

I know there is a new thread now with a different approach but I wanted
to say that this did fix that warning on the BeagleV Starlight beta
prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
starlight branch [3] which is a set of StarFive patches on top of
5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
warning on boot for me and the bootlog [5] shows:

  [    2.302598] Checked W+X mappings: passed, no W+X pages found

Thus if useful, here is my TB:

Tested-by: Drew Fustini <drew@beagleboard.org>

thanks,
drew

[1] https://github.com/beagleboard/beaglev-starlight
[2] https://github.com/starfive-tech/beaglev_doc
[3] https://github.com/esmil/linux/tree/starlight
[4] https://github.com/esmil/linux/commit/b865eb820db8b9b94a7a2d2619e46cc3251fb90d
[5] https://gist.github.com/pdp7/0bf9088d034384d29ba4e8418c9dfd91

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-20  4:55   ` Drew Fustini
  0 siblings, 0 replies; 12+ messages in thread
From: Drew Fustini @ 2021-05-20  4:55 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	linux-riscv, linux-kernel, Wei Fu, Emil Renner Berthing

On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
> From: Jisheng Zhang <jszhang@kernel.org>
> 
> When the kernel mapping was moved the last 2GB of the address space,
> (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> start address, the last set_memory_nx() in protect_kernel_text_data()
> will fail, thus the .data section is still mapped as W+X. This results
> in below W+X mapping waring at boot. Fix it by passing the correct
> .data section page num to the set_memory_nx().
> 
> [    0.396516] ------------[ cut here ]------------
> [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> [    0.398964] Modules linked in:
> [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> [    0.400591] epc : note_page+0x244/0x24a
> [    0.401368]  ra : note_page+0x244/0x24a
> [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.408052] Call Trace:
> [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> 
> Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/mm/init.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 4faf8bd157ea..4c4c92ce0bb8 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>  	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)));
> +#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, (max_low - data_start) >> PAGE_SHIFT);
> +	set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>  }
>  
>  void mark_rodata_ro(void)
> -- 
> 2.31.0
> 
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

I know there is a new thread now with a different approach but I wanted
to say that this did fix that warning on the BeagleV Starlight beta
prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
starlight branch [3] which is a set of StarFive patches on top of
5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
warning on boot for me and the bootlog [5] shows:

  [    2.302598] Checked W+X mappings: passed, no W+X pages found

Thus if useful, here is my TB:

Tested-by: Drew Fustini <drew@beagleboard.org>

thanks,
drew

[1] https://github.com/beagleboard/beaglev-starlight
[2] https://github.com/starfive-tech/beaglev_doc
[3] https://github.com/esmil/linux/tree/starlight
[4] https://github.com/esmil/linux/commit/b865eb820db8b9b94a7a2d2619e46cc3251fb90d
[5] https://gist.github.com/pdp7/0bf9088d034384d29ba4e8418c9dfd91

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
  2021-05-20  4:55   ` Drew Fustini
@ 2021-05-20  9:38     ` Jisheng Zhang
  -1 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-20  9:38 UTC (permalink / raw)
  To: Drew Fustini, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: Jisheng Zhang, linux-riscv, linux-kernel, Wei Fu, Emil Renner Berthing

On Wed, 19 May 2021 21:55:40 -0700
Drew Fustini <drew@beagleboard.org> wrote:

> 
> 
> On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
> > From: Jisheng Zhang <jszhang@kernel.org>
> >
> > When the kernel mapping was moved the last 2GB of the address space,
> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> > start address, the last set_memory_nx() in protect_kernel_text_data()
> > will fail, thus the .data section is still mapped as W+X. This results
> > in below W+X mapping waring at boot. Fix it by passing the correct
> > .data section page num to the set_memory_nx().
> >
> > [    0.396516] ------------[ cut here ]------------
> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> > [    0.398964] Modules linked in:
> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> > [    0.400591] epc : note_page+0x244/0x24a
> > [    0.401368]  ra : note_page+0x244/0x24a
> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.408052] Call Trace:
> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> >
> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  arch/riscv/mm/init.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4faf8bd157ea..4c4c92ce0bb8 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
> >       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)));
> > +#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, (max_low - data_start) >> PAGE_SHIFT);
> > +     set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
> >  }
> >
> >  void mark_rodata_ro(void)
> > --
> > 2.31.0
> >



> I know there is a new thread now with a different approach but I wanted
> to say that this did fix that warning on the BeagleV Starlight beta
> prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
> starlight branch [3] which is a set of StarFive patches on top of
> 5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
> warning on boot for me and the bootlog [5] shows:
> 
>   [    2.302598] Checked W+X mappings: passed, no W+X pages found
> 
> Thus if useful, here is my TB:
> 
> Tested-by: Drew Fustini <drew@beagleboard.org>
> 

Thank you. Alex's solution is better. It ensures there's no W+X mapping
from the beginning. So far, riscv's STRICT_KERNEL_RWX method is to create
W+X mapping from the beginning but remove W or X attribute at the end of kernel
booting. Alex's solution changes STRICT_KERNEL_RWX method, one effective side
of it is fixing this W+X mapping. I'm not sure whether Alex's patch should
be merged in this cycle or next window since rc1 is released. 

Two choice to fix:
Merge this small fix for linux-5.13 and bring Alex's patch for linux-next

Or

Use Alex's patch directly.


I would leave the decision to riscv maintainers.

Thanks

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-20  9:38     ` Jisheng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Jisheng Zhang @ 2021-05-20  9:38 UTC (permalink / raw)
  To: Drew Fustini, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
  Cc: Jisheng Zhang, linux-riscv, linux-kernel, Wei Fu, Emil Renner Berthing

On Wed, 19 May 2021 21:55:40 -0700
Drew Fustini <drew@beagleboard.org> wrote:

> 
> 
> On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
> > From: Jisheng Zhang <jszhang@kernel.org>
> >
> > When the kernel mapping was moved the last 2GB of the address space,
> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
> > start address, the last set_memory_nx() in protect_kernel_text_data()
> > will fail, thus the .data section is still mapped as W+X. This results
> > in below W+X mapping waring at boot. Fix it by passing the correct
> > .data section page num to the set_memory_nx().
> >
> > [    0.396516] ------------[ cut here ]------------
> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
> > [    0.398964] Modules linked in:
> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
> > [    0.400591] epc : note_page+0x244/0x24a
> > [    0.401368]  ra : note_page+0x244/0x24a
> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.408052] Call Trace:
> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
> >
> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  arch/riscv/mm/init.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 4faf8bd157ea..4c4c92ce0bb8 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
> >       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)));
> > +#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, (max_low - data_start) >> PAGE_SHIFT);
> > +     set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
> >  }
> >
> >  void mark_rodata_ro(void)
> > --
> > 2.31.0
> >



> I know there is a new thread now with a different approach but I wanted
> to say that this did fix that warning on the BeagleV Starlight beta
> prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
> starlight branch [3] which is a set of StarFive patches on top of
> 5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
> warning on boot for me and the bootlog [5] shows:
> 
>   [    2.302598] Checked W+X mappings: passed, no W+X pages found
> 
> Thus if useful, here is my TB:
> 
> Tested-by: Drew Fustini <drew@beagleboard.org>
> 

Thank you. Alex's solution is better. It ensures there's no W+X mapping
from the beginning. So far, riscv's STRICT_KERNEL_RWX method is to create
W+X mapping from the beginning but remove W or X attribute at the end of kernel
booting. Alex's solution changes STRICT_KERNEL_RWX method, one effective side
of it is fixing this W+X mapping. I'm not sure whether Alex's patch should
be merged in this cycle or next window since rc1 is released. 

Two choice to fix:
Merge this small fix for linux-5.13 and bring Alex's patch for linux-next

Or

Use Alex's patch directly.


I would leave the decision to riscv maintainers.

Thanks

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
  2021-05-20  9:38     ` Jisheng Zhang
@ 2021-05-29 20:38       ` Palmer Dabbelt
  -1 siblings, 0 replies; 12+ messages in thread
From: Palmer Dabbelt @ 2021-05-29 20:38 UTC (permalink / raw)
  To: Jisheng.Zhang
  Cc: drew, Paul Walmsley, aou, alex, jszhang3, linux-riscv,
	linux-kernel, tekkamanninja, emil.renner.berthing

On Thu, 20 May 2021 02:38:59 PDT (-0700), Jisheng.Zhang@synaptics.com wrote:
> On Wed, 19 May 2021 21:55:40 -0700
> Drew Fustini <drew@beagleboard.org> wrote:
>
>>
>>
>> On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
>> > From: Jisheng Zhang <jszhang@kernel.org>
>> >
>> > When the kernel mapping was moved the last 2GB of the address space,
>> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
>> > start address, the last set_memory_nx() in protect_kernel_text_data()
>> > will fail, thus the .data section is still mapped as W+X. This results
>> > in below W+X mapping waring at boot. Fix it by passing the correct
>> > .data section page num to the set_memory_nx().
>> >
>> > [    0.396516] ------------[ cut here ]------------
>> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
>> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
>> > [    0.398964] Modules linked in:
>> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
>> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
>> > [    0.400591] epc : note_page+0x244/0x24a
>> > [    0.401368]  ra : note_page+0x244/0x24a
>> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
>> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
>> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
>> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
>> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
>> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
>> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
>> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
>> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
>> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
>> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
>> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
>> > [    0.408052] Call Trace:
>> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
>> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
>> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
>> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
>> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
>> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
>> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
>> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
>> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
>> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
>> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
>> >
>> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
>> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>> > ---
>> >  arch/riscv/mm/init.c | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> > index 4faf8bd157ea..4c4c92ce0bb8 100644
>> > --- a/arch/riscv/mm/init.c
>> > +++ b/arch/riscv/mm/init.c
>> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>> >       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)));
>> > +#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, (max_low - data_start) >> PAGE_SHIFT);
>> > +     set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>> >  }
>> >
>> >  void mark_rodata_ro(void)
>> > --
>> > 2.31.0
>> >
>
>
>
>> I know there is a new thread now with a different approach but I wanted
>> to say that this did fix that warning on the BeagleV Starlight beta
>> prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
>> starlight branch [3] which is a set of StarFive patches on top of
>> 5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
>> warning on boot for me and the bootlog [5] shows:
>>
>>   [    2.302598] Checked W+X mappings: passed, no W+X pages found
>>
>> Thus if useful, here is my TB:
>>
>> Tested-by: Drew Fustini <drew@beagleboard.org>
>>
>
> Thank you. Alex's solution is better. It ensures there's no W+X mapping
> from the beginning. So far, riscv's STRICT_KERNEL_RWX method is to create
> W+X mapping from the beginning but remove W or X attribute at the end of kernel
> booting. Alex's solution changes STRICT_KERNEL_RWX method, one effective side
> of it is fixing this W+X mapping. I'm not sure whether Alex's patch should
> be merged in this cycle or next window since rc1 is released.
>
> Two choice to fix:
> Merge this small fix for linux-5.13 and bring Alex's patch for linux-next

I'm going to do this.  That way we keep can fix the bug now.  I've put 
this on a branch that I've merged into fixes, it looks like Alex is 
going to send a v3 so I'll wait on that.

Thanks!

>
> Or
>
> Use Alex's patch directly.
>
>
> I would leave the decision to riscv maintainers.
>
> Thanks

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

* Re: [PATCH] riscv: mm: Fix W+X mappings at boot
@ 2021-05-29 20:38       ` Palmer Dabbelt
  0 siblings, 0 replies; 12+ messages in thread
From: Palmer Dabbelt @ 2021-05-29 20:38 UTC (permalink / raw)
  To: Jisheng.Zhang
  Cc: drew, Paul Walmsley, aou, alex, jszhang3, linux-riscv,
	linux-kernel, tekkamanninja, emil.renner.berthing

On Thu, 20 May 2021 02:38:59 PDT (-0700), Jisheng.Zhang@synaptics.com wrote:
> On Wed, 19 May 2021 21:55:40 -0700
> Drew Fustini <drew@beagleboard.org> wrote:
>
>>
>>
>> On Sun, May 16, 2021 at 05:00:38PM +0800, Jisheng Zhang wrote:
>> > From: Jisheng Zhang <jszhang@kernel.org>
>> >
>> > When the kernel mapping was moved the last 2GB of the address space,
>> > (__va(PFN_PHYS(max_low_pfn))) is much smaller than the .data section
>> > start address, the last set_memory_nx() in protect_kernel_text_data()
>> > will fail, thus the .data section is still mapped as W+X. This results
>> > in below W+X mapping waring at boot. Fix it by passing the correct
>> > .data section page num to the set_memory_nx().
>> >
>> > [    0.396516] ------------[ cut here ]------------
>> > [    0.396889] riscv/mm: Found insecure W+X mapping at address (____ptrval____)/0xffffffff80c00000
>> > [    0.398347] WARNING: CPU: 0 PID: 1 at arch/riscv/mm/ptdump.c:258 note_page+0x244/0x24a
>> > [    0.398964] Modules linked in:
>> > [    0.399459] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #14
>> > [    0.400003] Hardware name: riscv-virtio,qemu (DT)
>> > [    0.400591] epc : note_page+0x244/0x24a
>> > [    0.401368]  ra : note_page+0x244/0x24a
>> > [    0.401772] epc : ffffffff80007c86 ra : ffffffff80007c86 sp : ffffffe000e7bc30
>> > [    0.402304]  gp : ffffffff80caae88 tp : ffffffe000e70000 t0 : ffffffff80cb80cf
>> > [    0.402800]  t1 : ffffffff80cb80c0 t2 : 0000000000000000 s0 : ffffffe000e7bc80
>> > [    0.403310]  s1 : ffffffe000e7bde8 a0 : 0000000000000053 a1 : ffffffff80c83ff0
>> > [    0.403805]  a2 : 0000000000000010 a3 : 0000000000000000 a4 : 6c7e7a5137233100
>> > [    0.404298]  a5 : 6c7e7a5137233100 a6 : 0000000000000030 a7 : ffffffffffffffff
>> > [    0.404849]  s2 : ffffffff80e00000 s3 : 0000000040000000 s4 : 0000000000000000
>> > [    0.405393]  s5 : 0000000000000000 s6 : 0000000000000003 s7 : ffffffe000e7bd48
>> > [    0.405935]  s8 : ffffffff81000000 s9 : ffffffffc0000000 s10: ffffffe000e7bd48
>> > [    0.406476]  s11: 0000000000001000 t3 : 0000000000000072 t4 : ffffffffffffffff
>> > [    0.407016]  t5 : 0000000000000002 t6 : ffffffe000e7b978
>> > [    0.407435] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003
>> > [    0.408052] Call Trace:
>> > [    0.408343] [<ffffffff80007c86>] note_page+0x244/0x24a
>> > [    0.408855] [<ffffffff8010c5a6>] ptdump_hole+0x14/0x1e
>> > [    0.409263] [<ffffffff800f65c6>] walk_pgd_range+0x2a0/0x376
>> > [    0.409690] [<ffffffff800f6828>] walk_page_range_novma+0x4e/0x6e
>> > [    0.410146] [<ffffffff8010c5f8>] ptdump_walk_pgd+0x48/0x78
>> > [    0.410570] [<ffffffff80007d66>] ptdump_check_wx+0xb4/0xf8
>> > [    0.410990] [<ffffffff80006738>] mark_rodata_ro+0x26/0x2e
>> > [    0.411407] [<ffffffff8031961e>] kernel_init+0x44/0x108
>> > [    0.411814] [<ffffffff80002312>] ret_from_exception+0x0/0xc
>> > [    0.412309] ---[ end trace 7ec3459f2547ea83 ]---
>> > [    0.413141] Checked W+X mappings: failed, 512 W+X pages found
>> >
>> > Fixes: 2bfc6cd81bd17e43 ("riscv: Move kernel mapping outside of linear mapping")
>> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>> > ---
>> >  arch/riscv/mm/init.c | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> > index 4faf8bd157ea..4c4c92ce0bb8 100644
>> > --- a/arch/riscv/mm/init.c
>> > +++ b/arch/riscv/mm/init.c
>> > @@ -746,14 +746,18 @@ void __init protect_kernel_text_data(void)
>> >       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)));
>> > +#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, (max_low - data_start) >> PAGE_SHIFT);
>> > +     set_memory_nx(data_start, (end_va - data_start) >> PAGE_SHIFT);
>> >  }
>> >
>> >  void mark_rodata_ro(void)
>> > --
>> > 2.31.0
>> >
>
>
>
>> I know there is a new thread now with a different approach but I wanted
>> to say that this did fix that warning on the BeagleV Starlight beta
>> prototype board [1] with the StarFive JH7100 SoC [2]. I am using Emil's
>> starlight branch [3] which is a set of StarFive patches on top of
>> 5.13-rc2. Emil included this W+X mapping patch [4]. It does fix the
>> warning on boot for me and the bootlog [5] shows:
>>
>>   [    2.302598] Checked W+X mappings: passed, no W+X pages found
>>
>> Thus if useful, here is my TB:
>>
>> Tested-by: Drew Fustini <drew@beagleboard.org>
>>
>
> Thank you. Alex's solution is better. It ensures there's no W+X mapping
> from the beginning. So far, riscv's STRICT_KERNEL_RWX method is to create
> W+X mapping from the beginning but remove W or X attribute at the end of kernel
> booting. Alex's solution changes STRICT_KERNEL_RWX method, one effective side
> of it is fixing this W+X mapping. I'm not sure whether Alex's patch should
> be merged in this cycle or next window since rc1 is released.
>
> Two choice to fix:
> Merge this small fix for linux-5.13 and bring Alex's patch for linux-next

I'm going to do this.  That way we keep can fix the bug now.  I've put 
this on a branch that I've merged into fixes, it looks like Alex is 
going to send a v3 so I'll wait on that.

Thanks!

>
> Or
>
> Use Alex's patch directly.
>
>
> I would leave the decision to riscv maintainers.
>
> Thanks

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, other threads:[~2021-05-29 20:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-16  9:00 [PATCH] riscv: mm: Fix W+X mappings at boot Jisheng Zhang
2021-05-16  9:00 ` Jisheng Zhang
2021-05-18 15:26 ` Alex Ghiti
2021-05-18 15:26   ` Alex Ghiti
2021-05-19 15:23   ` Jisheng Zhang
2021-05-19 15:23     ` Jisheng Zhang
2021-05-20  4:55 ` Drew Fustini
2021-05-20  4:55   ` Drew Fustini
2021-05-20  9:38   ` Jisheng Zhang
2021-05-20  9:38     ` Jisheng Zhang
2021-05-29 20:38     ` Palmer Dabbelt
2021-05-29 20:38       ` Palmer Dabbelt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.