From: Alexandre Ghiti <alex@ghiti.fr> To: "Björn Töpel" <bjorn@kernel.org>, "Paul Walmsley" <paul.walmsley@sifive.com>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Albert Ou" <aou@eecs.berkeley.edu>, linux-riscv@lists.infradead.org Cc: "Björn Töpel" <bjorn@rivosinc.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH] riscv: mm: Proper page permissions after initmem free Date: Mon, 14 Nov 2022 15:37:10 +0100 [thread overview] Message-ID: <b82f4580-76a0-e02b-05c8-4a89e576da0c@ghiti.fr> (raw) In-Reply-To: <20221112113543.3165646-1-bjorn@kernel.org> 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
WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Ghiti <alex@ghiti.fr> To: "Björn Töpel" <bjorn@kernel.org>, "Paul Walmsley" <paul.walmsley@sifive.com>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Albert Ou" <aou@eecs.berkeley.edu>, linux-riscv@lists.infradead.org Cc: "Björn Töpel" <bjorn@rivosinc.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH] riscv: mm: Proper page permissions after initmem free Date: Mon, 14 Nov 2022 15:37:10 +0100 [thread overview] Message-ID: <b82f4580-76a0-e02b-05c8-4a89e576da0c@ghiti.fr> (raw) In-Reply-To: <20221112113543.3165646-1-bjorn@kernel.org> 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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-11-14 14:37 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-12 11:35 [PATCH] riscv: mm: Proper page permissions after initmem free Björn Töpel 2022-11-12 11:35 ` Björn Töpel 2022-11-13 21:21 ` Samuel Holland 2022-11-13 21:21 ` Samuel Holland 2022-11-14 14:37 ` Alexandre Ghiti [this message] 2022-11-14 14:37 ` Alexandre Ghiti 2022-11-14 15:25 ` Björn Töpel 2022-11-14 15:25 ` Björn Töpel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b82f4580-76a0-e02b-05c8-4a89e576da0c@ghiti.fr \ --to=alex@ghiti.fr \ --cc=aou@eecs.berkeley.edu \ --cc=bjorn@kernel.org \ --cc=bjorn@rivosinc.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.