Align getdents and getdents64 records to fix unaligned access on MIPS#1636
Open
retrocpugeek wants to merge 3 commits into
Open
Align getdents and getdents64 records to fix unaligned access on MIPS#1636retrocpugeek wants to merge 3 commits into
retrocpugeek wants to merge 3 commits into
Conversation
getdents64 wrote linux_dirent64 records with d_reclen = header + name, without rounding up to the alignment of the leading u64 d_ino. The next record's d_ino then lands unaligned, and a strict-alignment guest (e.g. MIPS/MIPS64) faults with UC_ERR_READ_UNALIGNED when walking the buffer. x86 tolerates the unaligned load, which is why this was never caught. Round d_reclen up to the d_ino alignment, matching the kernel. The rounding is scoped to the getdents64 (is_64) branch only: getdents64 places d_type before d_name so the trailing pad bytes are inert, whereas the legacy getdents layout stores d_type at offset d_reclen-1 and would break if padded the same way -- that is what got the earlier blanket fix (PR qilingframework#1419) reverted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Calls ql_syscall_getdents64 on a directory and walks the returned linux_dirent64 records, asserting every record (and thus its leading u64 d_ino) starts on an 8-byte boundary. Before the fix d_reclen was not rounded up, so records were misaligned and a strict-alignment guest (e.g. MIPS) faulted with an unaligned load. Self-contained; runs on stock unicorn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
15 tasks
Extend the record alignment to the legacy getdents path. The kernel rounds both getdents and getdents64 records up to the alignment of their leading d_ino (ALIGN(reclen, sizeof(long))); without it a strict-alignment guest (MIPS) faults walking the buffer, e.g. older glibc busybox 'ls' (which uses the legacy getdents syscall) crashes on directory listings. Legacy linux_dirent stores d_type in the record's last byte (offset d_reclen-1), so the alignment padding goes between d_name and d_type to keep d_type there -- padding without relocating d_type is what got the earlier blanket attempt (PR qilingframework#1419) reverted. getdents64 (d_type before d_name) is unchanged. Add test_linux_getdents_alignment: walks legacy getdents records on a big-endian MIPS guest and asserts each is aligned, tiles the buffer, and keeps d_type (DT_DIR for '.'/'..'). Runs on stock unicorn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
retrocpugeek
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
__getdents_commondid not round dirent records up to the alignment of their leadingd_ino, so the next record'sd_inocould land unaligned. A strict-alignment guest (MIPS/MIPS64) then faults withUC_ERR_*_UNALIGNEDwhile walking the buffer (e.g.busybox lscrashes right after the syscall). x86 tolerates the unaligned access, which is why it was never caught.This affects both
getdents64(u64 d_ino) and the legacygetdents(word-sizedd_ino— 8 bytes on a 64-bit guest such as MIPS64 n64; older glibc still issues this syscall).Closes #1635.
How
Round each record up to the alignment of
d_ino, matching the kernel'sALIGN(reclen, sizeof(long)):applied to both record layouts. The one subtlety is
d_type:getdents64placesd_typebefored_name, so the trailing pad bytes are inert.getdentsstoresd_typein the record's last byte (offsetd_reclen - 1), so the alignment padding is inserted betweend_nameandd_typeto keepd_typethere.Relationship to #1419 / #1423 / #1425
This bug was fixed once (#1419) and reverted (#1423); #1425 re-submits
(d_reclen + n) & ~(n - 1)and is still open. That blanket approach had two issues this PR fixes properly:(d_reclen + n)over-pads — the canonical round-up is(d_reclen + (n - 1)).d_type, so the consumer'sd_typeread (atd_reclen - 1) landed in the pad bytes — almost certainly the revert cause. This PR movesd_typeto the newd_reclen - 1, so the legacy layout is correct after padding.This supersedes #1425.
Tests
test_elf.ELFTest.test_linux_getdents64_alignment— drivesql_syscall_getdents64on a directory and asserts everylinux_dirent64record starts 8-byte aligned.test_elf.ELFTest.test_linux_getdents_alignment— drives the legacyql_syscall_getdentsand asserts every record is aligned, tiles the buffer, and still carriesd_type(DT_DIRfor./..) in its last byte.Both are self-contained (no rootfs binary, run on stock unicorn); each fails on
devand passes with this fix.Checklist