squashfs: honor partition offset in Read and Finalize#404
Conversation
| fs := &FileSystem{ | ||
| workspace: "", // no workspace when we do nothing with it | ||
| start: start, | ||
| start: 0, // backend is already biased by start (if non-zero) via Sub above |
There was a problem hiding this comment.
I don't quite understand. The current approach is to take the backend "as is", save the start in the FileSystem{}, and then offset all calls with start. Why is it better to offset the backend and set the start to 0? Isn't it the same thing? And with this change, don't you need to remove all references to fs.start (and maybe remove it as the property entirely)? Or are you saying that the current approach should do that, but ignores fs.start anyways?
There was a problem hiding this comment.
Apparently there are about 15 places where start is not added in calls to ReadAt/WriteAt. But more challenging, there are two places where it must not be added (finalize.go:380 and finalize.go:678) since those are ReatAt's from the source.
Thus instead of doing this correctly at each call site it is a lot more clear to bias in the object so that the destination (fs.backend.Writable()) gets the start bias.
FWIW this was originally discovered using a test in partitionresizer, but adding a specific test in this PR to make it be self-contained.
There was a problem hiding this comment.
It turns out that the FileSystem.start was never read anywhere.
As an optional additional commit in this PR it can be removed.
| origHasher := sha256.New() | ||
| size, err := origPart.ReadContents(d.Backend, origHasher) | ||
| if err != nil { | ||
| if _, err := origPart.ReadContents(d.Backend, NewLimitWriter(origHasher, expectedSize)); err != nil { |
There was a problem hiding this comment.
These verify changes look fine, but really should be its own PR
There was a problem hiding this comment.
Moving them out to a separate PR.
squashfs.Create and squashfs.Read work entirely in offsets relative to the start of the filesystem, but the backend they were handed referred to the whole disk. When the filesystem lives inside a partition (a non-zero start), Finalize wrote every section — including the superblock at offset 0 — to absolute backend offsets, clobbering the protective MBR/GPT and placing the filesystem in the wrong spot, and Read dispatched its metadata-table reads at unbiased squashfs-internal offsets and read garbage. Wrap the backend in backend.Sub(b, start, size) at Create and Read time when start is non-zero, so every internal ReadAt/WriteAt is biased by the partition offset while the existing offset arithmetic stays unchanged. Whole-disk callers (start == 0) are unaffected. Add a regression test that builds a squashfs inside a GPT partition and reads it back; the existing tests only ever exercised start == 0. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eriknordmark
force-pushed
the
squashfs-partition-offset-and-verify-grow
branch
from
dd979d7 to
e7100c3
Compare
eriknordmark
changed the title
The FileSystem.start field was only ever written (to 0 once the offset fix wraps the backend in backend.Sub) and never read anywhere in the package, so it carried no information. Remove it. The partition offset now lives entirely in the Sub-wrapped backend; the start parameter of Create and Read is unchanged. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
squashfs: honor the partition offset in Read and Finalize
squashfs.Createandsquashfs.Readuse offsets relative to the startof the filesystem throughout (superblock at offset 0, then the fragment
/ inode / xattr tables at squashfs-internal offsets parsed from the
superblock). But the
backend.Storagethey were handed always referredto the whole disk. When the caller passes a non-zero
start— i.e. thefilesystem lives inside a partition:
Finalizewrote every section, including the superblock atWriteAt(sbBytes, 0), to absolute backend offsets, clobbering theprotective MBR / GPT and placing the squashfs in the wrong spot.
Readread the superblock atstartbut then dispatchedreadFragmentTable/readXattrsTable/readUidsGidsat unbiasedsquashfs-internal offsets, so on a non-zero start it read garbage
(
error reading fragments: ... unexpected EOF).This wraps the backend in
backend.Sub(b, start, size)atCreate- andRead-time whenstart != 0.Subauto-biases everyReadAt/WriteAtby the partition offset, so the existing internal offset arithmetic is
unchanged — no per-callsite edits. Whole-disk callers (
start == 0) areunaffected.
Test
Adds
TestSquashfsInPartition: it builds a squashfs inside a GPTpartition 1 MiB in, finalizes, asserts the GPT survived and the
superblock landed at the partition offset, then reopens and reads the
marker file back. It fails without this fix (superblock lands at offset
0); the existing whole-disk tests never caught this because they always
pass
start == 0.Notes
sync/verifyBlockCopychange that was originally bundled here hasbeen split into sync: allow target partition larger than source #406 per review feedback.
order.