Repository navigation
fix(raw): refuse to overwrite the partition table beside a mount - #461
Merged
Merged
Conversation
Writing a bootloader to the gap ahead of the first partition of a disk was refused whenever any partition of that disk was mounted, as is always the case for the running rootfs: target device "/dev/mmcblk1" is in use, it holds the filesystem mounted at "/" The raw handler now checks only the region it writes, so partitions of a whole disk lying outside of it are ignored. The disk itself, its holders, overlapping partitions and partitions with unreadable bounds are still checked, as is any other target. raw-delta keeps the exclusive check, as its written length is not known up front. Signed-off-by: Luciano Dittgen <luciano.dittgen@ossystems.com.br>
The in-use check covers the region an object declares, from its size or, when compressed, its required-uncompressed-size, but the write itself went on for as long as the source or the decompressor produced data. Metadata understating the data let the write run into a partition the check had left out. The write now stops at the checked region and fails the install when the data goes past it. A compressed object not declaring its uncompressed size has no known region, so it keeps the exclusive check and is not bounded. Signed-off-by: Luciano Dittgen <luciano.dittgen@ossystems.com.br>
The region check only had its pure helpers tested. Exercise it against a loop device holding a mounted partition after a 1 MiB gap: a write to the gap is allowed, while one reaching into the partition, the whole disk and the partition itself are rejected. Signed-off-by: Luciano Dittgen <luciano.dittgen@ossystems.com.br>
The copy and tarball tests attached their loop device by hand and detached it only at the end of a successful run, so every failure left one attached. A missing mkfs.ext4 alone leaked one per test. They now use FakeLoopDevice, which detaches on drop. FakeLoopDevice::with_partition went on silently when the partition node never showed up, leaving the caller to fail later on an unrelated error. It now fails on the spot, detaching the device. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
Bounding every write to the declared region turned understated metadata into failed installs. uhu takes required-uncompressed-size from `gzip -l`, which before gzip 1.12 reports it modulo 4 GiB, so images that always installed failed after partly writing their target. The bound also only stopped the write once it was under way, leaving the device partly written. The region only matters when the check let the write through beside a mounted partition, so only then is the write bounded. Whatever else is written whole, as before the region check. Beside a mount, the source is checked to fit before the device is opened, uncompressing it when compressed, and a larger one is refused untouched. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
Only the region check itself was exercised on a device, never the raw installer going through it. Lay out an am335x on a loop disk, its rootfs partition mounted from 1 MiB: MLO and U-Boot install in the gap, plain and compressed, without touching the partition; an object reaching into it is refused; and one holding more data than declared is refused without writing. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
The tests attaching loop devices format them as ext4, which failed for lack of mkfs.ext4 wherever the host did not happen to provide it. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
These tests are ignored by default, and CI never ran them, so the in-use and region checks were only exercised on hardware. Build the test binary as the runner and run just it as root, keeping the cached build owned by the runner. The MTD tests stay out, as they load simulator modules. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
A whole disk may be written beside its mounted partitions as long as the write misses them, but the partition tables lie outside every partition. Overwriting them left the mount working while the next boot found no partition at all. A write reaching the MBR, an EBR or a GPT of a disk with partitions is now refused while anything on that disk is mounted, as a write to the whole disk is. With nothing mounted, an image carrying the table still installs. Only what holds partitions in place is protected, so the boot loaders sharing that area keep installing: - MBR: bytes 440 to 512, leaving the boot code ahead of them to BIOS loaders such as GRUB. - EBR: every sector in the chain of an extended partition. - GPT: both headers and the whole entry arrays they declare, located from the headers in logical blocks, so tables shrunk or moved out of the way of a boot loader, as on Allwinner, are honoured. As the kernel does, a GPT is only looked for behind a protective or hybrid MBR. A table which cannot be read or makes no sense holds no partition in place and is left out, so disks partitioned from the kernel command line keep writing beside their mounts. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
FakeLoopDevice only offered a single MBR partition. Covering the partition tables needs GPT and extended layouts, several partitions and 4 KiB logical blocks, which loopdev cannot set, so the image is laid out by the caller and the block size set through LOOP_SET_BLOCK_SIZE, rescanning the partitions afterwards. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
Beside a mounted partition, writing the MBR, an EBR or either GPT, also with 4 KiB logical blocks, is refused, and allowed once nothing is mounted. The boot loader layouts sharing that area keep installing: BIOS boot code ahead of the MBR table, i.MX6 at 1 KiB, i.MX8M and Rockchip at 32 KiB, am335x from 128 KiB, and Allwinner's SPL at 8 KiB beside a GPT whose entries were shrunk or moved out of its way. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
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
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.
Follow-up to #460, and stacked on it: the first eight commits are #460's, only the last three are new. Please review those three, and merge after #460.
Problem
#460 lets a
rawobject write a whole disk beside its mounted partitions, as long as the write misses them. The partition tables lie outside every partition, so this also allowed overwriting them. On a loop disk with ext4 mounted from p1, zeroing MBR sector 0, or GPT LBA 0–33 and the last 33 sectors, was refused on master but allowed by #460. The mount kept working, butsfdisk -dfound no table and, afterblockdev --rereadpt(what the next boot sees), p1 was gone.Fix
A write reaching the partition table of a disk with partitions is refused while anything on that disk is mounted, with a new
PartitionTableInUseerror. With nothing mounted, an image carrying the table still installs. A write reaching a mounted partition still fails withDeviceInUse, as before.Only the bytes that keep partitions in place are protected:
boot.imgor syslinuxmbr.binkeep writing the boot code ahead of them.queue/logical_block_size). Allwinner GPT layouts that shrink or move the entries out of the SPL's way at 8 KiB keep working. As the kernel does, a GPT is only looked for behind a protective or hybrid MBR, so a stale GPT header on an MBR disk does not block i.MX6 U-Boot at 1 KiB. The backup is looked for where the primary says, or on the last sector when the primary is unusable.A table that cannot be read or makes no sense holds no partition in place, so it is left out with a warning. Disks partitioned from the kernel command line (
blkdevparts=) keep #460's behaviour.The disk is read through the target path, read-only, with no new dependencies.
Writes that leave the table bytes unchanged are refused like any other overlap, as on master.
Tests
rawinstaller, the original reproduction is refused, and a 440-byte boot code installs beside a mount, leaving the table intact.FakeLoopDevice::with_imageattaches any image, setting 4 KiB logical blocks throughLOOP_SET_BLOCK_SIZE, sinceloopdevcannot.cargo fmt --check,cargo clippy --all-features --all --tests -- -D clippy::all, the workspace tests and the root tests all pass locally, with no loop devices or mounts left behind.