linux-kernel - Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5365422a9715376c76a89e255c978fc39064e243.camel@physik.fu-berlin.de>
Date: Tue, 11 Mar 2025 18:28:21 +0100
From: John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
To: Artur Rojek <contact@...ur-rojek.eu>, Yoshinori Sato	
 <ysato@...rs.sourceforge.jp>, Rich Felker <dalias@...c.org>, Daniel Lezcano
	 <daniel.lezcano@...aro.org>, Thomas Gleixner <tglx@...utronix.de>, Uros
 Bizjak	 <ubizjak@...il.com>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>, "D . Jeff Dionne"	
 <jeff@...esemi.io>, Rob Landley <rob@...dley.net>,
 linux-sh@...r.kernel.org, 	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] sh: align .bss section padding to 8-byte boundary

Hi Artur,

I'm currently trying to review this patch, but I'm not 100% sure how it
this change helps grows the .bss section, see below. Maybe you can help
me understand what's happening.

On Sun, 2025-02-16 at 18:55 +0100, Artur Rojek wrote:
> J2 based devices expect to find a devicetree blob at the end of the bss
> section. As of a77725a9a3c5, libfdt enforces 8-byte alignment for the
> dtb, causing J2 devices to fail early in sh_fdt_init.
> 
> As J2 loader firmware calculates the dtb location based on the kernel
> image .bss section size, rather than the __bss_stop symbol offset, the
> required alignment can't be enforced with BSS_SECTION(0, PAGE_SIZE, 8).
> Instead, inline modified version of the above macro, which grows .bss
> by the required size.
> 
> While this change affects all existing SH boards, it should be benign on
> platforms which don't need this alignment.
> 
> Signed-off-by: Artur Rojek <contact@...ur-rojek.eu>
> ---
>  arch/sh/kernel/vmlinux.lds.S | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
> index 9644fe187a3f..008c30289eaa 100644
> --- a/arch/sh/kernel/vmlinux.lds.S
> +++ b/arch/sh/kernel/vmlinux.lds.S
> @@ -71,7 +71,20 @@ SECTIONS
>  
>  	. = ALIGN(PAGE_SIZE);
>  	__init_end = .;
> -	BSS_SECTION(0, PAGE_SIZE, 4)
> +	__bss_start = .;
> +	SBSS(0)
> +	. = ALIGN(PAGE_SIZE);

What this effectively does is removing ". = ALIGN(sbss_align);" first from BSS_SECTION().

Then it inserts ". = ALIGN(PAGE_SIZE);" after the "SBSS(0)".

If I understand this correctly, SBSS() inserts a zero-padding and if I'm not mistaken,
inserting ". = ALIGN(PAGE_SIZE);" will cause this padding to grow to at least PAGE_SIZE
due the alignment.

Is this correct?

> +	.bss : AT(ADDR(.bss) - LOAD_OFFSET) {
> +		BSS_FIRST_SECTIONS
> +		. = ALIGN(PAGE_SIZE);
> +		*(.bss..page_aligned)
> +		. = ALIGN(PAGE_SIZE);
> +		*(.dynbss)
> +		*(BSS_MAIN)
> +		*(COMMON)
> +		. = ALIGN(8);

If my understanding above is correct, why do we will need an additional ". = ALIGN(8)"
here?

> +	}
> +	__bss_stop = .;
>  	_end = . ;
>  
>  	STABS_DEBUG

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913