Messages in this thread | | | Date | Mon, 14 Nov 2022 15:37:10 +0100 | Subject | Re: [PATCH] riscv: mm: Proper page permissions after initmem free | From | Alexandre Ghiti <> |
| |
Hi Björn,
On 12/11/2022 12:35, Björn Töpel wrote: > From: Björn Töpel <bjorn@rivosinc.com> > > 64-bit RISC-V kernels have the kernel image mapped separately, and in > addition to the linear map. When the kernel is loaded, the linear map > of kernel image is set to PAGE_READ permission, and the kernel map is > set to PAGE_READ and PAGE_EXEC. > > When the initmem is freed, the corresponding pages in the linear map > should be restored to PAGE_READ and PAGE_WRITE. The corresponding > pages in the kernel map should also be restored to PAGE_READ and > PAGE_WRITE, by removing the PAGE_EXEC permission, and adding > PAGE_WRITE. > > This is not the case. For 64-bit kernels, only the linear map is > restored to its proper page permissions at initmem free, and not the > kernelmap. > > In practise this results in that the kernel can potentially jump to > dead __init code, and start executing invalid 0xcc instructions, > without getting an exception. > > Restore the freed initmem properly, by setting both the alias (kernel > map) and the linear map to the correct permissions. > > Fixes: e5c35fa04019 ("riscv: Map the kernel with correct permissions the first time") > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> > --- > arch/riscv/kernel/setup.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index ad76bb59b059..361e635070fe 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -321,10 +321,12 @@ subsys_initcall(topology_init); > > void free_initmem(void) > { > - if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) > - set_kernel_memory(lm_alias(__init_begin), lm_alias(__init_end), > - IS_ENABLED(CONFIG_64BIT) ? > - set_memory_rw : set_memory_rw_nx); > + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { > + if (IS_ENABLED(CONFIG_64BIT)) > + set_kernel_memory(lm_alias(__init_begin), lm_alias(__init_end), > + set_memory_rw); > + set_kernel_memory(__init_begin, __init_end, set_memory_rw_nx);
That's nits but for 64-bit kernels, do we really want to make the kernel mapping writable? I think catching a write access here would be better because IMO that should not happen.
Thanks,
Alex
> + } > > free_initmem_default(POISON_FREE_INITMEM); > } > > base-commit: 442bcbfd2c5401587b983e34bed0b407214735c3
| |