Skip to content

lzloader: give .rodata its own output section#1405

Open
gburd wants to merge 1 commit into
cloudius-systems:masterfrom
gburd:pr/lzloader-rodata
Open

lzloader: give .rodata its own output section#1405
gburd wants to merge 1 commit into
cloudius-systems:masterfrom
gburd:pr/lzloader-rodata

Conversation

@gburd

@gburd gburd commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Split out of #1399 per review.

This was the unrelated linker-script change flagged in review on the io_uring
commit (arch/x64/lzloader.ld:13). It gives .rodata its own output section in
the lzloader (kernel decompressor) link and is not part of the io_uring change,
so it is submitted on its own here.

Build-qualified (kernel compile+link, image=empty) on a binutils 2.44 /
g++ 14.3.0 host.

The decompressor stub's linker script placed .data at a fixed
OSV_LZKERNEL_BASE + 0x3000 offset and never emitted an output
section for .rodata, so read-only data (string literals, jump
tables) emitted by the compressor stub had no defined home and was
silently folded into the gap before .data.  Emit an explicit
.rodata output section between .text and .data so the stub's
read-only data is laid out deterministically instead of depending
on the 0x3000 fudge factor.
@nyh

nyh commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@wkozaczuk can you please review this patch? It makes sense and I don't know where this 0x3000 (which you added many years ago in commit baf5c30) comes from. The same commit has other explanations for the number 0x3000 (three pages) so I am not sure why it was really there, or if the other mentions 0f 0x3000 in that commit are justified.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the x64 lzloader linker script to explicitly place read-only data into its own output section, separating it from writable data during the kernel decompressor link.

Changes:

  • Add an explicit .rodata output section to capture .rodata and .rodata.* input sections.
  • Remove the fixed location-counter jump (OSV_LZKERNEL_BASE + 0x3000) that previously separated .text from subsequent sections.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gburd

gburd commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

To answer where the 0x3000 comes from, since it bears on whether dropping it here is safe:

It's not arbitrary. It was introduced in baf5c301 (the almost-in-place decompression work) and it's the size reserved for the decompressor stub at the front of lzloader.elf. The authoritative coupling is in fastlz/fastlz.h:

/* ... first 3 pages (0x3000) in lzloader.elf are occupied by fastlz
 * decompression code and next 4 bytes store offset of the segments
 * info table. ... */
#define MAX_COMPRESSED_SEGMENT_SIZE (SEGMENT_SIZE - sizeof(int) - 0x3000)

So the decompressor's worst-case overlap math assumes the stub (its own code + read-only + writable data) fits within the first 0x3000 bytes of the image, before the segment-info table and the compressed payload. That constant is the thing that must not change casually — and this patch does not touch it.

What the old linker line actually did, and why it's fragile: the script never gave .rodata an explicit output section. .rodata was therefore an orphan, and its placement relative to the manual . = OSV_LZKERNEL_BASE + 0x3000 bump and .data is left to ld's orphan-placement heuristics, which differ across binutils versions. If .rodata is placed past .edata, it falls outside the region the stub copies, and any read-only data the decompressor references is wrong. The . = base + 0x3000 assignment only forced .data to the 3-page boundary; it did nothing to pin .rodata.

This patch gives .rodata its own output section immediately after .text, so it is deterministically part of the copied stub prefix, and drops the now-redundant . = base + 0x3000 bump (it was only separating .text from .data).

The fastlz.h reservation is preserved: in a local build with this change the loadable stub (.text + .rodata) ends at file offset ~0x2ba8, comfortably inside the first three pages, and .data/.bss/.edata follow as before. Kernel builds and links clean (binutils 2.44 / g++ 14.3, image=empty).

@wkozaczuk — you wrote the original; does this match your intent for the 0x3000 reservation? If you'd rather keep an explicit assertion that the stub stays under 0x3000 (e.g. ASSERT(. <= OSV_LZKERNEL_BASE + 0x3000, "lzloader stub exceeds reserved 3 pages") before .data), I'm happy to add it so the coupling to fastlz.h is enforced at link time rather than by convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants