Skip to content

Fix BSS segment corruption in file-backed mmap (dynamic library loading)#331

Merged
Pryancito merged 1 commit into
mainfrom
copilot/init-gui-service
Apr 30, 2026
Merged

Fix BSS segment corruption in file-backed mmap (dynamic library loading)#331
Pryancito merged 1 commit into
mainfrom
copilot/init-gui-service

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 30, 2026

When fmap handles a file-backed mapping where memsz > filesz (BSS extension), it was allocating physical pages based on min(len, file_size - offset) — the file-data portion only — while sys_mmap mapped len-worth of pages using that physical address. BSS pages fell outside the allocation, mapping to adjacent unallocated physical frames with stale content. BSS globals initialized to zero by the ABI held garbage values, causing null-pointer-ish dereferences during shared library init.

Observed crash: lightdm (dynamic binary) faulted at CR2=0x7ce8 with RAX=0x7cd8 — a GLib/GObject BSS global used as a pointer, non-zero due to this bug.

Changes

  • filesystem.rsfmap for OpenFile::Real: compute aligned_len from the full requested len rather than want. The pre-existing write_bytes(..., 0, aligned_len) already zeros the whole region; only the file-data slice (want bytes) is read from disk — no change to the read path.
// Before: allocates pages only for file-data portion, BSS maps to garbage
let aligned_len = (want + 0xFFF) & !0xFFF;

// After: allocates pages for full mapping (file data + BSS extension)
let aligned_len = (len + 0xFFF) & !0xFFF;

@Pryancito Pryancito marked this pull request as ready for review April 30, 2026 12:15
Copilot AI review requested due to automatic review settings April 30, 2026 12:15
@Pryancito Pryancito merged commit 8614629 into main Apr 30, 2026
1 check passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect physical backing for file-backed mmap when the requested mapping length (len) exceeds the file-data portion (filesz, i.e., memsz > filesz BSS extension). This prevents BSS pages from mapping to unrelated/unallocated physical frames containing stale data, which can crash dynamic library initialization.

Changes:

  • In FileSystemScheme::fmap for OpenFile::Real, compute aligned_len from the full requested len (not from want).
  • Clarify comments around want (file bytes) vs len (file bytes + BSS extension) and the zeroing/copy behavior.

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

let max_len = file_size - offset;
let want = core::cmp::min(len, max_len);
let aligned_len = (want + 0xFFF) & !0xFFF;
let aligned_len = (len + 0xFFF) & !0xFFF;
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

aligned_len is computed with (len + 0xFFF) & !0xFFF, which can overflow/wrap for very large len values (release builds wrap). Since len ultimately comes from userspace mmap, this can lead to allocating/zeroing the wrong amount of memory. Use a checked/saturating align-up (e.g., len.checked_add(0xFFF) + mask) and return EINVAL on overflow.

Suggested change
let aligned_len = (len + 0xFFF) & !0xFFF;
let Some(aligned_len) = len.checked_add(0xFFF).map(|v| v & !0xFFF) else {
return Err(scheme_error::EINVAL);
};

Copilot uses AI. Check for mistakes.
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