fix(filesystem): zero-fill newly exposed file ranges - #100
Conversation
srpatcha
left a comment
There was a problem hiding this comment.
Approving. This is an information-disclosure fix, and the test earns its place.
The defect
eos_fs_truncate() zeroed on shrink and not on growth. So the sequence
write("secret"); truncate(0); truncate(6); read()returns secret. The bytes were never cleared — only n->size moved — so
shrinking a file and growing it back republishes whatever was there. Anything
that treats truncation as deletion is wrong on this filesystem, and the naming in
your test (/secret, "must not make discarded contents readable again") is the
right framing: this is disclosure, not a correctness nit.
Verified
Applied on current master:
0 build errors
100% tests passed, 0 tests failed out of 28
Then reverted only the growth branch, restoring the shrink-only form:
test_filesystem.c:81: test_fs_truncate_growth_zero_fills_stale_bytes:
Assertion `memcmp(buf, zeros, sizeof(buf)) == 0' failed.
0% tests passed, 1 tests failed out of 1
Fails on the unfixed code, passes on the fixed one. That is what separates a
regression test from a demonstration, and it is worth saying because two tests I
have looked at this week did not manage it — one asserted on values correct
before and after the bug, and another (mine) used an input that happened not to
trigger the defect.
The seek-past-EOF gap fill is the same class caught in the same pass:
if (p > n->size) memset(n->data + n->size, 0, p - n->size);Writing after seeking beyond EOF previously left whatever was in the buffer
visible in the gap.
On the evidence row
Thank you for stating what you could not verify:
Host CTest launch is unverified because the Windows cross-build emits
extensionless PE test paths.
That is more useful than an unqualified "all tests pass", and it let me spend my
verification where it mattered rather than re-checking what you had already
covered. For what it is worth, host CTest does run cleanly on Linux — 28/28 — so
that gap is a Windows toolchain artefact rather than anything about this change.
Also correct to point at #95 for the separate write-length overflow rather than
folding it in. That one is approved and covers p + len > n->cap wrapping when
len is near SIZE_MAX. Both touch filesystem.c; whichever lands second will
need a trivial rebase, and there is no semantic disagreement between them.
Summary
The RAM filesystem retained old bytes in its fixed inode buffer after
EOS_O_TRUNC. Growing that file witheos_fs_truncate(), or seeking past EOFand writing, made those discarded bytes readable again instead of exposing
zero-filled storage.
This change zero-fills every newly readable range while preserving existing
capacity clamping, shrinking, and ordinary read/write behavior.
Type of Change
Changes
were discarded with
EOS_O_TRUNC.Testing
-Wall -Wextra -Werror15/15 passed10 passedPre-Submission Checklist
-Wall -Wextra -Werrorfor C)<type>(<scope>): <description>conventionmasterRelated Issues
None.
Screenshots / Logs
Not applicable.
Additional Notes
extensionless paths for PE test executables; the focused binary was run
directly instead.
eos_fs_write()is alreadyaddressed by fix(filesystem): avoid write length wrap when clamping #95. This PR deliberately does not duplicate that change,
though the adjacent edits may cause a trivial merge conflict.
unchanged.