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 |
