diff mbox

arm64: Align .text section to PAGE_SIZE

Message ID 1445610134-20528-1-git-send-email-jeremy.linton@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeremy Linton Oct. 23, 2015, 2:22 p.m. UTC
It appears that 64k page kernel's die early, in a somewhat random set
of locations when built without KVM. Most likely during memblock
manipulations (depending on kernel debug options).

Normally when KVM is built into the kernel it has an explicit
PAGE_SIZE alignment requirement and that forces the text section to be
aligned to PAGE_SIZE. Without it, the alignment granularity is likely to
be 4k.

This updates the linker script to assure that the the text section is
aligned to a minimum of PAGE_SIZE regardless of build options.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/vmlinux.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ard Biesheuvel Oct. 23, 2015, 3:34 p.m. UTC | #1
Hi Jeremy,

On 23 October 2015 at 16:22, Jeremy Linton <jeremy.linton@arm.com> wrote:
> It appears that 64k page kernel's die early, in a somewhat random set

nit: kernels

> of locations when built without KVM. Most likely during memblock
> manipulations (depending on kernel debug options).
>

It looks like fixup_executable() [with DEBUG_RODATA enabled] is the
culprit here: it calls create mapping() with a size that gets rounded
up to a multiple of PAGE_SIZE, which means the region that ends up
getting its exec bits cleared is larger than it should. Perhaps it is
better to fix that instead?

> Normally when KVM is built into the kernel it has an explicit
> PAGE_SIZE alignment requirement and that forces the text section to be
> aligned to PAGE_SIZE. Without it, the alignment granularity is likely to
> be 4k.
>
> This updates the linker script to assure that the the text section is
> aligned to a minimum of PAGE_SIZE regardless of build options.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  arch/arm64/kernel/vmlinux.lds.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 8a5d97b..bf7b972 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -92,7 +92,7 @@ SECTIONS
>                 HEAD_TEXT
>         }
>         ALIGN_DEBUG_RO
> -       .text : {                       /* Real text segment            */
> +       .text ALIGN(PAGE_SIZE) : {      /* Real text segment            */
>                 _stext = .;             /* Text and read-only data      */
>                         __exception_text_start = .;
>                         *(.exception.text)
> --
> 2.4.3
>
>
Jeremy Linton Oct. 23, 2015, 4:32 p.m. UTC | #2
On 10/23/2015 10:34 AM, Ard Biesheuvel wrote:
> It looks like fixup_executable() [with DEBUG_RODATA enabled] is the
> culprit here: it calls create mapping() with a size that gets rounded
> up to a multiple of PAGE_SIZE, which means the region that ends up
> getting its exec bits cleared is larger than it should. Perhaps it is
> better to fix that instead?

Ard,
	Thanks for taking a look at this!

	Your probably right.

	But AFAIK, with just that fix text/ro sections can be partially mapped 
without ro. It seems to me that putting a little padding in so that they 
are on separate pages is a good idea, especially since it seems the 
remaining sections of interest are page aligned, either explicitly in 
the linker script or via assembly directives in assorted places.

	Maybe a clearer fix is an additional ifdef in the DEBUG_ALIGN_RODATA 
stanza which sets ALIGN_DEBUG_RO to align to pagesize if DEBUG_RODATA is 
set, that way it only takes affect for RODATA.

Thanks,
	Jeremy
diff mbox

Patch

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 8a5d97b..bf7b972 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -92,7 +92,7 @@  SECTIONS
 		HEAD_TEXT
 	}
 	ALIGN_DEBUG_RO
-	.text : {			/* Real text segment		*/
+	.text ALIGN(PAGE_SIZE) : {	/* Real text segment		*/
 		_stext = .;		/* Text and read-only data	*/
 			__exception_text_start = .;
 			*(.exception.text)