sysupgrade: verify the rootfs before flashing the kernel, and bound the verify-mount

[wkumik]

Problem

do_update_rootfs() loop-mounts the new rootfs to read its SoC stamp and version before writing it. The check itself is reasonable — but its failure path has three problems, and together they can strand a device.

1. It runs too late — a failure leaves the device half-flashed

The kernel is flashed first, and the rootfs is verified after:

[ "1" = "$update_kernel" ] && do_update_kernel "$kernel_file"
[ "1" = "$update_rootfs" ] && do_update_rootfs "$rootfs_file"   # verification lives in here

So a rootfs that fails verification is only discovered once the kernel has already been committed. The device reboots with a new kernel on an old rootfs and does not recover by itself. The user sees a flash that "didn't take", with no obvious reason.

2. It can hang forever

The mount does not always fail on a rootfs the running kernel cannot read — it can block indefinitely. And there is no output at all between the echo and the flash write:

echo "Update rootfs from $x"                                    # ← last line the user ever sees
if mkdir -p "$y" && loop=$(losetup -f) && losetup "$loop" "$x" && mount "$loop" "$y"; then

Everything in that condition is silent, so a wedged mount is indistinguishable from a dead tool: the upgrade stops with no error, no timeout, and no way to tell what happened. Any GUI driving sysupgrade goes down with it.

We hit this on an SSC338Q air unit. Verbatim, from the flashing tool's log:

20:40:38  Kernel updated to 06:49:22 2026-06-15     <- kernel written + verified (2024/2024 kb)
20:40:38  RootFS
20:40:38  Update rootfs from /tmp/rootfs.squashfs.ssc338q
          <<< zero further output, ever >>>

It never emitted another byte. Note do_update_kernel() has no mount and goes straight to flashcp, which is exactly why the kernel writes fine and streams progress while the rootfs produces nothing — the asymmetry is structural.

3. It is fatal when it need not be

die "Unable to mount $y!" rejects the flash. But an unmountable rootfs is usually not a defect in the image — it means the running kernel lacks the squashfs decompressor the new image uses (commonly XZ). That is a property of the kernel being replaced, and it stops mattering the moment the new kernel boots. The mount is only a pre-check: flashcp writes the partition raw and never needs it. So a perfectly good image is refused on a criterion that doesn't apply to it.

Fix

Split verification out of the write into verify_rootfs(), and:

• Run it before the first flashcp. A rootfs problem now costs nothing — nothing has been written, and the device stays on its working firmware. This alone removes the half-flashed state.
• Bound the mount with timeout (falling back to a bare mount where the applet is absent), so the pre-flight check can never outlive the flash it guards.
• Warn instead of die when the image cannot be mounted, and let flashcp write it.

The SoC check is preserved

Mounting the rootfs is how /etc/hostname is read to confirm the image matches the SoC, so it isn't dropped lightly. When the mount fails, soc_guarded_by_kernel() asks whether the run has any other SoC witness:

• A kernel with a readable SoC is flashed in the same run → the uImage header carries the SoC and check_soc validates it with no mount at all. Warn and proceed.
• No such witness, but the image was downloaded (-k/-r/--url/--channel/--archive) → the artifact is pinned to $model by the name it must have to be found at all (rootfs.squashfs.$model, md5-verified). Warn and proceed.
• No witness and a hand-picked --rootfs= file → nothing pins it, and writing an unverified rootfs for a foreign SoC bricks the device. This case still refuses — before anything is written — and tells the user to flash the matching kernel in the same run or pass --force_soc.

Note that "a kernel is being flashed" is not by itself a SoC witness, which the first version of this PR assumed: a FIT kernel carries no SoC field (do_update_kernel skips the probe), and check_soc is skipped outright for ingenic/rockchip. Those now count as no-witness rather than being waved through. Every path is equal-or-better than master, which flashed that same unverified kernel first and only then died.

Same-version skip, --skip_ver, --force_soc and the combined-image path all behave as before (do_update_firmware verifies before its split write too, passing kernel_follows since it flashes the kernel unconditionally straight after).

Testing

sh -n and dash -n parse clean. The behavioural harness is committed as .github/scripts/test_sysupgrade.sh and wired into the existing shell-tests.yml next to test_load_hisilicon.sh. It runs the real script against a stubbed device — no QEMU, no root, a few seconds — and every failure case asserts the kernel was never written.

19/19 pass on this branch. More usefully, pointing it at master (SRC=<master copy> bash .github/scripts/test_sysupgrade.sh) puts 13 checks red, and the flash log in each shows flashcp -v .../uImage.ssc338q /dev/mtd2 already recorded at the point of death — the half-flash, reproduced:

scenario	expected	master	this PR
mount OK, new version	verify → flash kernel → flash rootfs	✅	✅
mount OK, same version	rootfs write skipped	✅	✅
mount OK, wrong SoC	refuse, write nothing	❌ kernel already written	✅
verify precedes the first write	ordering	❌ no verify before kernel	✅
mount fails, kernel in same run	warn, flash both	❌ dies, kernel written	✅
mount fails	surface the real mount error	❌ discarded	✅
mount hangs	bounded near mount_wait	❌ unbounded	✅ ~3s
mount fails, rootfs-only	refuse, write nothing	✅	✅
mount fails, FIT kernel, local files	refuse (no SoC witness)	❌ kernel written	✅
mount fails, ingenic, local files	refuse (no SoC witness)	❌ kernel written	✅
mount fails, FIT kernel, downloaded	proceed (name-pinned)	❌ dies	✅
refusal advises a real option	--force_soc	—	✅

scr_version bumped to 1.0.54.

[wkumik]

Gentle bump on this one — CI is green and it still applies cleanly. I appreciate everyone's time is thin, so I'll keep this to what's new since I opened it rather than restating the PR.

This is still actively biting people. As of yesterday (15 July), the peer-to-peer advice being passed around for flashing an SSC338Q air unit is to give up on the tooling entirely and do it by hand:

python -m http.server 8000        # on the PC
wget http://<pc>:8000/rootfs.squashfs.ssc338q     # on the device, over SSH
wget http://<pc>:8000/uImage.ssc338q
flashcp -v /tmp/uImage.ssc338q /dev/mtd2
flashcp -v /tmp/rootfs.squashfs.ssc338q /dev/mtd3
flash_eraseall /dev/mtd4
reboot

That's a user independently arriving at "bypass sysupgrade, write the partitions raw." Worth noticing what the workaround is: it's the same flashcp calls sysupgrade already makes. Nobody is working around the write — they're working around the verify-mount in front of it, which is exactly the failure path this PR fixes.

The related client-side fix landed. OpenIPC/companion#190 was merged into dev — Companion will now report a failed flash instead of showing "Upgrade Complete!!" over one that never happened. That's a real improvement, but it only makes the failure visible; the flash still doesn't succeed. The three defects here are the reason it fails in the first place:

1. the rootfs is verified after do_update_kernel has already committed the kernel, so a rootfs problem leaves a half-upgraded device rather than a harmless no-op;
2. the mount can hang rather than fail, and there's no output between echo "Update rootfs from $x" and the write — so a wedged mount is indistinguishable from a dead tool (this is the "stuck at 60% forever" report; 60% is precisely the kernel-done / rootfs-starting boundary);
3. die "Unable to mount" rejects images that flashcp would have written correctly — the check that stops the flash stops applying the moment the new kernel boots.

The change is 68/9 on a single file, keeps the SoC check intact (mount-fail + rootfs-only still dies, but before any write), and I validated it with sh -n/dash -n plus a 12-case behavioural harness covering mount ok / same-version / fail / hang / rootfs-only, asserting the kernel is never written in any failure case.

I notice the repo already has a shell-test pattern (.github/scripts/test_load_hisilicon.sh wired into shell-tests.yml). If it would help review, I'm happy to contribute that harness as .github/scripts/test_sysupgrade.sh so the hang and half-flash cases are covered in CI rather than sitting in my working tree — just say the word and I'll push it onto this branch.

Also glad to split this into smaller commits, or to drop (3) and land only the "verify before you write" and timeout parts if the warn-instead-of-die change is the contentious one. The half-flash on failure is the piece I'd most like to see fixed regardless of what happens to the rest.

[widgetii]

Reviewed against master — the core claims check out, and this also quietly fixes an unreported instance of the same half-flash bug in the combined path (master's do_update_firmware flashed the FIT kernel before the rootfs check could reject; now verify_rootfs runs right after the split). Details inline; summary:

• Must-fix: the new die message advises --skip_soc, which is not an option (line 138).
• Should-fix: "SoC is validated from the kernel image" does not hold for FIT kernels or ingenic/rockchip (lines 130-140) — still net-better than master there, but the message overpromises.
• Minor: -r on a combined-image platform refuses while --url proceeds on identical evidence (line 186); plus diagnosability/output nits.

Verified along the way: CONFIG_TIMEOUT=y in the main busybox config, so the applet is present on real firmware (only the initramfs config lacks it, where sysupgrade doesn't run; the command -v fallback covers it); busybox 1.36 accepts the positional timeout SECS syntax; rootfs_version/exit_update wiring is sound on every reachable path; same-version skip and -f/force semantics are preserved; loop/mountpoint cleanup is equal-or-better than the old umount && rm && losetup -d chain; self_update's grep scr_version | head -1 still resolves line 3 first.

One ask beyond the inline notes: please commit the 12-case harness. .github/workflows/shell-tests.yml already runs .github/scripts/test_load_hisilicon.sh, so there is exact precedent — this failure matrix (especially the "kernel was never written" assertions) is the kind that regresses silently.

[widgetii]

--skip_soc is not an option. The parser accepts -f | --force_all | --force_md5 | --force_soc | --force_ver (line 547); --skip_soc falls through to the *) case → Unknown option → exit. A user following this advice mid-recovery gets rejected by the tool that sent them there. Should read --force_soc (or -f). Same typo in the PR description.

[widgetii]

This reassurance doesn't hold everywhere:

• FIT kernels (d00dfeed — exactly the combined cv6xx/hi3519dv500 images do_update_firmware feeds into verify_rootfs): do_update_kernel skips the uImage SoC probe (lines 75-76), and its own comment says it relies on "the rootfs hostname SoC check" — i.e. the check that just failed here.
• ingenic / rockchip: check_soc is skipped for those vendors (line 79).

In those cases this branch proceeds with no SoC validation at all. To be fair, that is still net equal-or-better than master — the old code flashed that same unverified kernel first and only then died — so not a regression. But the message and the comment above overpromise, and the next reader will reason from them.

Minimal fix: reword to "validated from the kernel image where possible" and note the FIT/ingenic/rockchip exception. Tighter option: probe the kernel file here — if it carries no probeable uImage SoC (FIT magic, excluded vendor), treat it like the rootfs-only case and refuse unless forced. Trade-off: that would refuse a legitimate manifest-downloaded cv6xx upgrade, where the artifact name (firmware.bin.$model) already pins the model — so if you tighten, maybe only for local --kernel=/--rootfs= files (remote_update unset).

[widgetii]

sysupgrade -r on a combined-image platform reaches here with update_kernel unset, so an unmountable rootfs takes the refuse-branch — "Flash the matching kernel in the same run" — even though this function unconditionally flashes the kernel on the very next line. Meanwhile --url/--channel (which set both flags) proceed on identical SoC evidence. Same evidence, opposite outcomes, and the advice is a no-op here.

Suggest making it explicit, e.g. verify_rootfs "/tmp/rootfs.squashfs.$model" kernel_follows with verify_rootfs treating $2 like update_kernel=1. (Avoid the update_kernel=1 verify_rootfs env-prefix idiom — in POSIX sh, assignments prefixed to a function call persist in the caller.)

[widgetii]

Two notes, neither blocking:

• CONFIG_TIMEOUT=y is set in general/package/busybox/busybox.config, so the applet exists on real firmware (only the initramfs config lacks it, where sysupgrade doesn't run) — the fallback is the right belt-and-braces.
• timeout sends TERM and busybox then waits to reap the child, so a mount wedged in uninterruptible sleep (D state) survives it and the hang remains. The realistic failures — missing decompressor → fast error, slow loop probe — are bounded, so best-effort is fine; just soften "Never let the pre-flight check outlive the flash" in the comment above, since this can't fully guarantee that.

[widgetii]

The 2>/dev/null here (and inside mount_rootfs) discards the actual error text the old code surfaced, and the warn block then guesses "likely lacks the squashfs decompressor" — which also masks loop-device exhaustion, a truncated download, etc. Consider capturing it (err=$(mount ... 2>&1)) and echoing it in the warn block; on a bricked-upgrade support thread, that one line is the difference between a diagnosis and a shrug.

Also consider -t squashfs -o ro for the verify-mount — it skips kernel fs autoprobing of a potentially garbage file.

[widgetii]

Nit: the verify output (SoC OK, New version, going to update) now prints before the Kernel section with no header of its own. A small echo_c 33 "\nVerify" + echo "Verifying rootfs from $x" here would keep tool transcripts self-explanatory — and it restores the "last line before a stall" breadcrumb this PR's description complains about: if the bounded mount does stall for the full mount_wait, the transcript currently goes silent at the previous section again.

[wkumik]

Thank you — this is a genuinely useful review. The independent verification (busybox CONFIG_TIMEOUT in the main config but not initramfs, 1.36's positional timeout syntax, the rootfs_version/exit_update wiring, self_update's grep | head -1 still resolving line 3) saved me re-deriving all of it, and you found something I'd missed entirely — the same half-flash in the combined path. Pushed as 2ec211f + 1ba46b9, stacked rather than force-pushed so your inline comments stay anchored.

--skip_soc (line 138) — must-fix. Fixed, in the message and in the PR description. You're right that it's the worst possible place to get it wrong: the tool sends you to a flag that makes the tool exit.

"SoC is validated from the kernel image" (lines 130-140). You're right, and I took the tighter option rather than rewording — the error was in the code, not just the prose. I'd assumed "a kernel is being flashed" implies "the SoC gets checked", which is exactly what your FIT/ingenic/rockchip note disproves.

New soc_guarded_by_kernel() asks whether the run actually has a SoC witness, i.e. a legacy uImage on a probed vendor. FIT and ingenic/rockchip now count as no witness instead of being waved through. Then, per your trade-off:

• no witness, but remote_update → warn and proceed: the artifact is pinned to $model by the name it must have to be found at all (rootfs.squashfs.$model, md5-verified), so a legitimate manifest-downloaded cv6xx upgrade isn't refused;
• no witness and a hand-picked --rootfs= → refuse, before any write.

Scoped to remote_update exactly as you suggested. No path here is worse than master, which flashed that same unverified kernel first and died afterwards.

-r on a combined-image platform (line 186). Fixed. To be precise about what fixes it: the refusal is now unreachable for combined images because they only ever arrive via a remote_update path (download_firmware is the only producer of firmware.bin.$model), so they take the name-pinned branch and the contradictory advice is gone. I still added the explicit verify_rootfs "..." kernel_follows you proposed — it's belt-and-braces today rather than load-bearing, but it states the intent at the call site and survives a reordering or a future local combined path. Thanks also for the env-prefix warning; I'd have reached for update_kernel=1 verify_rootfs and it would have leaked into the caller.

timeout / D-state (line 96). Softened. It now says best-effort and names what it does and doesn't bound. "Never let the pre-flight check outlive the flash" was a promise TERM can't keep.

Discarded error text (line 116). Captured and printed — including losetup's, so loop-device exhaustion surfaces as itself instead of as a guess about decompressors. Took -t squashfs -o ro as well.

Verify header (line 107). Added.

The harness

Committed as .github/scripts/test_sysupgrade.sh, wired into shell-tests.yml next to test_load_hisilicon.sh. 22 checks, pure shell, no QEMU or root, ~40s in CI.

The green run isn't the interesting number. Pointing it at master — SRC=<master copy> bash .github/scripts/test_sysupgrade.sh — puts 15 red, and the flash log in each failure shows flashcp -v .../uImage.ssc338q /dev/mtd2 already recorded at the point of death. That's the half-flash, reproduced rather than asserted.

That includes the combined-path bug you found. Your note exposed a real gap: nothing built a firmware.bin.$model, so image_combined was never set and do_update_firmware never ran — the one path you found the extra bug in was the one path the suite didn't touch. 1ba46b9 adds it, and against master:

FAIL combined path must verify before the FIT write -- verify@none kernel@39
FAIL combined + wrong SoC -> expected refusal with no write, rc=1
     log='flashcp -v .../uImage.ssc338q /dev/mtd2'

One caveat worth stating plainly: an earlier revision of this harness passed everything while a sed rule was quietly blanking every version read, which turned the same-version test green for the wrong reason. I only caught it by running it against master. So I'd treat the red run as the evidence the file is worth having, and I'd rather you trust that than the 22/22.

Full master-vs-PR matrix is in the PR description. Happy to split either commit if you'd prefer it in smaller pieces, and if the warn-instead-of-die part is still contentious I'm glad to drop it and land only "verify before you write" + the timeout — the half-flash is the piece I care about most.

[widgetii]

Approving. Verified the follow-up independently rather than re-reading it: ran the committed harness on a second machine — PR head passes clean, and pointed at master it puts up the same 15 red with flashcp .../uImage.ssc338q /dev/mtd2 already in the flash log at each point of death. The half-flash is reproduced, not asserted, and the combined-path gap found in review is now covered by its own failing-on-master test.

soc_guarded_by_kernel() is the right resolution — it mirrors do_update_kernel's actual skips instead of assuming them, and the remote_update scoping keeps name-pinned downloads working while refusing only the genuinely unguarded hand-picked-file case, before anything is written. No need to split the commits, and warn-instead-of-die should stay: it's properly guarded now, and no path is worse than master.