From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Ariel Marcovitch <arielmarcovitch@gmail.com>, mpe@ellerman.id.au Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, Ariel Marcovitch <ariel.marcovitch@gmail.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH] powerpc: fix alignment bug whithin the init sections Date: Fri, 18 Dec 2020 16:38:46 +0100 [thread overview] Message-ID: <4716e80b-db6f-7669-684f-398971ed5f2e@csgroup.eu> (raw) In-Reply-To: <20201213183556.16976-1-ariel.marcovitch@gmail.com> Le 13/12/2020 à 19:35, Ariel Marcovitch a écrit : > This is a bug that can cause early crashes in configurations with a > .exit.text section smaller than a page and a .init.text section that > ends in the beginning of a physical page (this is kinda random, which > might explain why this wasn't really encountered before). It can cause, or it causes ? Did you encounter the issue ? > > The init sections are ordered like this: > .init.text > .exit.text > .init.data > > Currently, these sections aren't page aligned. > > Because the init code is mapped read-only at runtime and because the > .init.text section can potentially reside on the same physical page as > .init.data, the beginning of .init.data might be mapped read-only along > with .init.text. init code is mapped PAGE_KERNEL_TEXT. Whether PAGE_KERNEL_TEXT is read-only or not depends on the selected options. > > Then when the kernel tries to modify a variable in .init.data (like > kthreadd_done, used in kernel_init()) the kernel panics. > > To avoid this, I made these sections page aligned. Should write this unpersonal, something like "To avoid this, make these sections page aligned" > > Fixes: 060ef9d89d18 ("powerpc32: PAGE_EXEC required for inittext") > Signed-off-by: Ariel Marcovitch <ariel.marcovitch@gmail.com> > --- > arch/powerpc/kernel/vmlinux.lds.S | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S > index 326e113d2e45..e3a7c90c03f4 100644 > --- a/arch/powerpc/kernel/vmlinux.lds.S > +++ b/arch/powerpc/kernel/vmlinux.lds.S > @@ -179,6 +179,11 @@ SECTIONS > #endif > } :text > > + /* .init.text is made RO and .exit.text is not, so we must > + * ensure these sections reside in separate physical pages. > + */ > + . = ALIGN(PAGE_SIZE); > + In principle, as it is text, it should be made RO as well. But what happens at the begining doesn't really matter, anyway .exit.text should never be executed and is discarded together with init text. So, I think it is OK the live with it as is for the time being. Making it page aligned makes sense anyway. Should we make _einittext page aligned instead, just like _etext ? > /* .exit.text is discarded at runtime, not link time, > * to deal with references from __bug_table > */ > @@ -186,6 +191,8 @@ SECTIONS > EXIT_TEXT > } > > + . = ALIGN(PAGE_SIZE); > + Here for sure, as you explain in the coming log, this needs to be separated from init text. So making it aligned is a must. > .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) { > INIT_DATA > } > > base-commit: 1398820fee515873379809a6415930ad0764b2f6 > Christophe
WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu> To: Ariel Marcovitch <arielmarcovitch@gmail.com>, mpe@ellerman.id.au Cc: linuxppc-dev@lists.ozlabs.org, paulus@samba.org, linux-kernel@vger.kernel.org, Ariel Marcovitch <ariel.marcovitch@gmail.com> Subject: Re: [PATCH] powerpc: fix alignment bug whithin the init sections Date: Fri, 18 Dec 2020 16:38:46 +0100 [thread overview] Message-ID: <4716e80b-db6f-7669-684f-398971ed5f2e@csgroup.eu> (raw) In-Reply-To: <20201213183556.16976-1-ariel.marcovitch@gmail.com> Le 13/12/2020 à 19:35, Ariel Marcovitch a écrit : > This is a bug that can cause early crashes in configurations with a > .exit.text section smaller than a page and a .init.text section that > ends in the beginning of a physical page (this is kinda random, which > might explain why this wasn't really encountered before). It can cause, or it causes ? Did you encounter the issue ? > > The init sections are ordered like this: > .init.text > .exit.text > .init.data > > Currently, these sections aren't page aligned. > > Because the init code is mapped read-only at runtime and because the > .init.text section can potentially reside on the same physical page as > .init.data, the beginning of .init.data might be mapped read-only along > with .init.text. init code is mapped PAGE_KERNEL_TEXT. Whether PAGE_KERNEL_TEXT is read-only or not depends on the selected options. > > Then when the kernel tries to modify a variable in .init.data (like > kthreadd_done, used in kernel_init()) the kernel panics. > > To avoid this, I made these sections page aligned. Should write this unpersonal, something like "To avoid this, make these sections page aligned" > > Fixes: 060ef9d89d18 ("powerpc32: PAGE_EXEC required for inittext") > Signed-off-by: Ariel Marcovitch <ariel.marcovitch@gmail.com> > --- > arch/powerpc/kernel/vmlinux.lds.S | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S > index 326e113d2e45..e3a7c90c03f4 100644 > --- a/arch/powerpc/kernel/vmlinux.lds.S > +++ b/arch/powerpc/kernel/vmlinux.lds.S > @@ -179,6 +179,11 @@ SECTIONS > #endif > } :text > > + /* .init.text is made RO and .exit.text is not, so we must > + * ensure these sections reside in separate physical pages. > + */ > + . = ALIGN(PAGE_SIZE); > + In principle, as it is text, it should be made RO as well. But what happens at the begining doesn't really matter, anyway .exit.text should never be executed and is discarded together with init text. So, I think it is OK the live with it as is for the time being. Making it page aligned makes sense anyway. Should we make _einittext page aligned instead, just like _etext ? > /* .exit.text is discarded at runtime, not link time, > * to deal with references from __bug_table > */ > @@ -186,6 +191,8 @@ SECTIONS > EXIT_TEXT > } > > + . = ALIGN(PAGE_SIZE); > + Here for sure, as you explain in the coming log, this needs to be separated from init text. So making it aligned is a must. > .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) { > INIT_DATA > } > > base-commit: 1398820fee515873379809a6415930ad0764b2f6 > Christophe
next prev parent reply other threads:[~2020-12-18 15:39 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-13 18:35 [PATCH] powerpc: fix alignment bug whithin the init sections Ariel Marcovitch 2020-12-13 18:35 ` Ariel Marcovitch 2020-12-18 15:38 ` Christophe Leroy [this message] 2020-12-18 15:38 ` Christophe Leroy 2020-12-18 20:35 ` ariel marcovitch 2020-12-18 20:43 ` ariel marcovitch 2020-12-18 20:43 ` ariel marcovitch
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=4716e80b-db6f-7669-684f-398971ed5f2e@csgroup.eu \ --to=christophe.leroy@csgroup.eu \ --cc=ariel.marcovitch@gmail.com \ --cc=arielmarcovitch@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ /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.