mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: BAREBOX <barebox@lists.infradead.org>,
	"Claude Sonnet 4.5" <noreply@anthropic.com>
Subject: Re: [PATCH 14/19] ARM: linker script: create separate PT_LOAD segments for text, rodata, and data
Date: Tue, 6 Jan 2026 00:01:49 +0100	[thread overview]
Message-ID: <aVxC3ZOkqe3lB6OI@pengutronix.de> (raw)
In-Reply-To: <1d0e01f8-a1d7-4c8c-982a-be21f9dcb67b@pengutronix.de>

On Mon, Jan 05, 2026 at 02:11:28PM +0100, Ahmad Fatoum wrote:
> 
> 
> On 1/5/26 12:26 PM, Sascha Hauer wrote:
> > Fix the linker scripts to generate three distinct PT_LOAD segments with
> > correct permissions instead of combining .rodata with .data.
> > 
> > Before this fix, the linker auto-generated only two PT_LOAD segments:
> > 1. Text segment (PF_R|PF_X)
> > 2. Data segment (PF_R|PF_W) - containing .rodata, .data, .bss, etc.
> 
> Did it though? Why did we get the RWX linker warnings then?
> 
> > This caused .rodata to be mapped with write permissions when
> > pbl_mmu_setup_from_elf() set up MMU permissions based on ELF segments,
> > defeating the W^X protection that commit d9ccb0cf14 intended to provide.
> 
> What commit hash is this?

It's an earlier variant of "ARM: PBL: setup MMU with proper permissions
from ELF segments". With this I originally had the problem that I could
write to the rodata section. This patch is the solution Claude came up
with, so Claude referenced it by the commit hash. I re-ordered the
patches afterwards.

> 
> > With explicit PHDRS directives, we now generate three segments:
> > 1. text segment (PF_R|PF_X): .text and related code sections
> > 2. rodata segment (PF_R): .rodata and unwind tables
> > 3. data segment (PF_R|PF_W): .data, .bss, and related sections
> 
> Not directly related, but this is as good a place as any to ask the
> question: How is zero-padding implemented? If the file size is shorter
> than the memory size, the loader is supposed to zero-fill, which is used
> for BSS zeroing for example. Now if you load the ELF in place, we can't
> obviously zero-fill.

Yes we can, see my other mail. The bss section is placed in the ELF binary,
unless it is at the very end of the binary in which case it becomes
smaller by the size of the bss section.

> > +PHDRS
> > +{
> > +	text PT_LOAD FLAGS(5);     /* PF_R | PF_X */
> > +	rodata PT_LOAD FLAGS(4);   /* PF_R */
> > +	data PT_LOAD FLAGS(6);     /* PF_R | PF_W */
> > +	dynamic PT_DYNAMIC FLAGS(6); /* PF_R | PF_W */
> 
> I believe we don't need PF_W for PT_DYNAMIC. You could move it one up to
> merge with rodata.

See readelf output:

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0x00000000 0x00000000 0x96190 0x96190 R E 0x1000
  LOAD           0x098000 0x00097000 0x00097000 0x4d2f4 0x4d2f4 R   0x1000
  LOAD           0x0e6000 0x000e5000 0x000e5000 0x216d4 0x283b0 RW  0x1000
  DYNAMIC        0x0f1d74 0x000f0d74 0x000f0d74 0x15960 0x15960 RW  0x4

The DYNAMIC segment is inside the LOAD segment directly above. I don't
know how this segment is really used, but making it readonly means that
it at least would have to be page aligned which it isn't.

> >  	_edata = .;
> > -	.image_end : { *(.__image_end) }
> > +	.image_end : { *(.__image_end) } :data
> >  
> >  	. = ALIGN(4);
> > -	.__bss_start :  { *(.__bss_start) }
> > -	.bss : { *(.bss*) }
> > -	.__bss_stop :  { *(.__bss_stop) }
> > +	.__bss_start :  { *(.__bss_start) } :data
> > +	.bss : { *(.bss*) } :data
> > +	.__bss_stop :  { *(.__bss_stop) } :data
> 
> Side-effect of having the decompressor take care of zeroing BSS is a big
> size increase for CONFIG_IMAGE_COMPRESSION_NONE. I think that's
> acceptable, but it's worth a comment here why we don't have a separate
> BSS segment (or make bss NOALLOC).

The bss section is not part of the binary when it's at its very end,
which it is, so I don't see any size increase.

> FYI, I have patches to get rid of MAX_BSS_SIZE on top of PBL ELF loader
> support, which I will submit once this support is merged.

Great \o/

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



  reply	other threads:[~2026-01-05 23:02 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-05 11:26 [PATCH 00/19] PBL: Add PBL ELF loading support with dynamic relocations Sascha Hauer
2026-01-05 11:26 ` [PATCH 01/19] elf: Use memcmp to make suitable for PBL Sascha Hauer
2026-01-05 11:46   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 02/19] elf: build for PBL as well Sascha Hauer
2026-01-05 11:26 ` [PATCH 03/19] elf: add dynamic relocation support Sascha Hauer
2026-01-05 14:05   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 04/19] ARM: implement elf_apply_relocations() for ELF " Sascha Hauer
2026-01-05 11:58   ` Ahmad Fatoum
2026-01-05 19:53     ` Sascha Hauer
2026-01-05 11:26 ` [PATCH 05/19] riscv: " Sascha Hauer
2026-01-05 11:26 ` [PATCH 06/19] elf: implement elf_load_inplace() Sascha Hauer
2026-01-05 13:37   ` Ahmad Fatoum
2026-01-05 22:42     ` Sascha Hauer
2026-01-06  8:18       ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 07/19] elf: create elf_open_binary_into() Sascha Hauer
2026-01-05 11:26 ` [PATCH 08/19] Makefile: add barebox.elf build target Sascha Hauer
2026-01-05 12:22   ` Ahmad Fatoum
2026-01-05 15:43     ` Sascha Hauer
2026-01-05 17:11       ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 09/19] PBL: allow to link ELF image into PBL Sascha Hauer
2026-01-05 12:11   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 10/19] mmu: add MAP_CACHED_RO mapping type Sascha Hauer
2026-01-05 12:14   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 11/19] mmu: introduce pbl_remap_range() Sascha Hauer
2026-01-05 12:15   ` Ahmad Fatoum
2026-01-06  8:50     ` Ahmad Fatoum
2026-01-06  9:25       ` Sascha Hauer
2026-01-05 11:26 ` [PATCH 12/19] ARM: use relative jumps in exception table Sascha Hauer
2026-01-05 11:44   ` Ahmad Fatoum
2026-01-05 12:29     ` Sascha Hauer
2026-01-05 12:31       ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 13/19] ARM: exceptions: make in-binary exception table const Sascha Hauer
2026-01-05 11:26 ` [PATCH 14/19] ARM: linker script: create separate PT_LOAD segments for text, rodata, and data Sascha Hauer
2026-01-05 13:11   ` Ahmad Fatoum
2026-01-05 23:01     ` Sascha Hauer [this message]
2026-01-06  7:59       ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 15/19] ARM: link ELF image into PBL Sascha Hauer
2026-01-05 12:27   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 16/19] ARM: PBL: setup MMU with proper permissions from ELF segments Sascha Hauer
2026-01-05 12:58   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 17/19] riscv: link ELF image into PBL Sascha Hauer
2026-01-05 13:12   ` Ahmad Fatoum
2026-01-05 11:26 ` [PATCH 18/19] riscv: linker script: create separate PT_LOAD segments for text, rodata, and data Sascha Hauer
2026-01-05 13:40   ` Ahmad Fatoum
2026-01-05 11:27 ` [PATCH 19/19] riscv: add ELF segment-based memory protection with MMU Sascha Hauer
2026-01-05 13:58   ` Ahmad Fatoum
2026-01-05 14:08 ` [PATCH 00/19] PBL: Add PBL ELF loading support with dynamic relocations Ahmad Fatoum
2026-01-05 16:47   ` Sascha Hauer
2026-01-06  8:35     ` Ahmad Fatoum

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=aVxC3ZOkqe3lB6OI@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=noreply@anthropic.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox