Re: [PATCH 2/2] powerpc/64s/radix: Fix RWX mapping with relocated kernel

2023-01-11 Thread Sachin Sant


> On 10-Jan-2023, at 6:17 PM, Michael Ellerman  wrote:
> 
> If a relocatable kernel is loaded at a non-zero address and told not to
> relocate to zero (kdump or RELOCATABLE_TEST), the mapping of the
> interrupt code at zero is left with RWX permissions.
> 
> That is a security weakness, and leads to a warning at boot if
> CONFIG_DEBUG_WX is enabled:
> 
>  powerpc/mm: Found insecure W+X mapping at address 
> 056435bc/0xc000
>  WARNING: CPU: 1 PID: 1 at arch/powerpc/mm/ptdump/ptdump.c:193 
> note_page+0x484/0x4c0
>  CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
> 6.2.0-rc1-1-g8ae8e98aea82-dirty #175
>  Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 
> 0xf05 of:SLOF,git-dd0dca hv:linux,kvm pSeries
>  NIP:  c04a1c34 LR: c04a1c30 CTR: 
>  REGS: c3503770 TRAP: 0700   Not tainted  
> (6.2.0-rc1-1-g8ae8e98aea82-dirty)
>  MSR:  82029033   CR: 24000220  XER: 
> 
>  CFAR: c0545a58 IRQMASK: 0
>  ...
>  NIP note_page+0x484/0x4c0
>  LR  note_page+0x480/0x4c0
>  Call Trace:
>note_page+0x480/0x4c0 (unreliable)
>ptdump_pmd_entry+0xc8/0x100
>walk_pgd_range+0x618/0xab0
>walk_page_range_novma+0x74/0xc0
>ptdump_walk_pgd+0x98/0x170
>ptdump_check_wx+0x94/0x100
>mark_rodata_ro+0x30/0x70
>kernel_init+0x78/0x1a0
>ret_from_kernel_thread+0x5c/0x64
> 
> The fix has two parts. Firstly the pages from zero up to the end of
> interrupts need to be marked read-only, so that they are left with R-X
> permissions. Secondly the mapping logic needs to be taught to ensure
> there is a page boundary at the end of the interrupt region, so that the
> permission change only applies to the interrupt text, and not the region
> following it.
> 
> Fixes: c55d7b5e6426 ("powerpc: Remove STRICT_KERNEL_RWX incompatibility with 
> RELOCATABLE")
> Signed-off-by: Michael Ellerman 
> ---

Thanks Michael. This fixes the problem reported earlier

https://lore.kernel.org/linuxppc-dev/48206911-fd3d-401a-a69d-1a79403e7...@linux.ibm.com/

Reported-by: Sachin Sant 
Tested-by: Sachin Sant 

- Sachin

[PATCH 2/2] powerpc/64s/radix: Fix RWX mapping with relocated kernel

2023-01-10 Thread Michael Ellerman
If a relocatable kernel is loaded at a non-zero address and told not to
relocate to zero (kdump or RELOCATABLE_TEST), the mapping of the
interrupt code at zero is left with RWX permissions.

That is a security weakness, and leads to a warning at boot if
CONFIG_DEBUG_WX is enabled:

  powerpc/mm: Found insecure W+X mapping at address 
056435bc/0xc000
  WARNING: CPU: 1 PID: 1 at arch/powerpc/mm/ptdump/ptdump.c:193 
note_page+0x484/0x4c0
  CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc1-1-g8ae8e98aea82-dirty 
#175
  Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1202 0xf05 
of:SLOF,git-dd0dca hv:linux,kvm pSeries
  NIP:  c04a1c34 LR: c04a1c30 CTR: 
  REGS: c3503770 TRAP: 0700   Not tainted  
(6.2.0-rc1-1-g8ae8e98aea82-dirty)
  MSR:  82029033   CR: 24000220  XER: 
  CFAR: c0545a58 IRQMASK: 0
  ...
  NIP note_page+0x484/0x4c0
  LR  note_page+0x480/0x4c0
  Call Trace:
note_page+0x480/0x4c0 (unreliable)
ptdump_pmd_entry+0xc8/0x100
walk_pgd_range+0x618/0xab0
walk_page_range_novma+0x74/0xc0
ptdump_walk_pgd+0x98/0x170
ptdump_check_wx+0x94/0x100
mark_rodata_ro+0x30/0x70
kernel_init+0x78/0x1a0
ret_from_kernel_thread+0x5c/0x64

The fix has two parts. Firstly the pages from zero up to the end of
interrupts need to be marked read-only, so that they are left with R-X
permissions. Secondly the mapping logic needs to be taught to ensure
there is a page boundary at the end of the interrupt region, so that the
permission change only applies to the interrupt text, and not the region
following it.

Fixes: c55d7b5e6426 ("powerpc: Remove STRICT_KERNEL_RWX incompatibility with 
RELOCATABLE")
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c 
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 5a2384ed1727..26245aaf12b8 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -234,6 +234,14 @@ void radix__mark_rodata_ro(void)
end = (unsigned long)__end_rodata;
 
radix__change_memory_range(start, end, _PAGE_WRITE);
+
+   for (start = PAGE_OFFSET; start < (unsigned long)_stext; start += 
PAGE_SIZE) {
+   end = start + PAGE_SIZE;
+   if (overlaps_interrupt_vector_text(start, end))
+   radix__change_memory_range(start, end, _PAGE_WRITE);
+   else
+   break;
+   }
 }
 
 void radix__mark_initmem_nx(void)
@@ -268,6 +276,11 @@ static unsigned long next_boundary(unsigned long addr, 
unsigned long end)
 
// Relocatable kernel running at non-zero real address
if (stext_phys != 0) {
+   // The end of interrupts code at zero is a rodata boundary
+   unsigned long end_intr = __pa_symbol(__end_interrupts) - 
stext_phys;
+   if (addr < end_intr)
+   return end_intr;
+
// Start of relocated kernel text is a rodata boundary
if (addr < stext_phys)
return stext_phys;
-- 
2.39.0