Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
widgetii merged 3 commits into from
Jul 17, 2026
Merged

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

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abaa77c
sysupgrade: verify the rootfs before flashing the kernel, and bound t…
wkumik Jul 11, 2026
2ec211f
sysupgrade: address review, and cover the failure matrix in CI
wkumik Jul 17, 2026
1ba46b9
test_sysupgrade: cover the combined-image path too
wkumik Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 68 additions & 9 deletions general/overlay/usr/sbin/sysupgrade
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#!/bin/sh
# OpenIPC.org | 2025
scr_version=1.0.53
scr_version=1.0.54

args="$@"
LOCK_FILE=/tmp/sysupgrade.lock
# Seconds the rootfs verify-mount may take before it is treated as unmountable.
mount_wait=${mount_wait:-45}
# Route to firmware or builder manifest. A stock OpenIPC/firmware build
# stamps BUILD_PLATFORM=${soc}_${variant} with variant in {lite,ultimate,
# neo}. Anything else — a per-device override (${soc}_${variant}_${device},
Expand Down Expand Up @@ -84,24 +86,74 @@ do_update_kernel() {
echo_c 32 "Kernel updated to $(get_kernel_version "$kernel_device")"
}

do_update_rootfs() {
# mount, bounded. The verify-mount does not always fail on a rootfs the running
# kernel cannot read — it can block indefinitely. Nothing is printed between the
# mount and the flash write, so an unbounded mount that wedges is indistinguishable
# from a dead tool: the upgrade stops forever with no error, taking any GUI driving
# it down too. Never let the pre-flight check outlive the flash it guards.
mount_rootfs() {
if command -v timeout >/dev/null 2>&1; then
timeout "$mount_wait" mount "$1" "$2" 2>/dev/null

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

else
mount "$1" "$2" 2>/dev/null
fi
}

# Read the candidate rootfs's SoC stamp and version by loop-mounting it.
#
# This is a PRE-FLIGHT CHECK ONLY: the image is written with flashcp, which writes
# the partition raw and never needs the mount. So it must run BEFORE anything is
# written to flash, and it must not reject an image that would have flashed fine.
verify_rootfs() {

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

local x=$1
[ -z "$x" ] && x="/tmp/rootfs.squashfs.$model"
echo_c 33 "\nRootFS"
echo "Update rootfs from $x"
[ ! -f "$x" ] && die "File $x not found"

local y=/tmp/rootfs
if mkdir -p "$y" && loop=$(losetup -f) && losetup "$loop" "$x" && mount "$loop" "$y"; then
local loop=
rootfs_version=

if mkdir -p "$y" && loop=$(losetup -f) && losetup "$loop" "$x" 2>/dev/null \

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

&& mount_rootfs "$loop" "$y"; then
rootfs_version=$(get_system_version "$y")
check_soc "$(head -1 $y/etc/hostname | cut -d- -f2)"
compare_versions "$system_version" "$rootfs_version" && exit_update=1
umount "$y" && rm -rf "$y" && losetup -d "$loop"
[ "1" = "$exit_update" ] && return 0
umount "$y"
else
die "Unable to mount $y!"
# Unmountable is not a defect in the image. It means the RUNNING kernel
# lacks the squashfs decompressor the NEW image uses (commonly XZ) — a
# property of the kernel being replaced, and irrelevant the moment the new
# one boots. Aborting here rejects a perfectly good image, so warn instead.
echo_c 33 "\nCannot mount $x to verify it."
echo_c 33 "The running kernel likely lacks its squashfs decompressor; the image itself may be fine."

# The SoC is also stamped in the uImage header and validated with no mount
# at all, so a kernel flashed in this same run still guards against a
# wrong-SoC image. Flashing the rootfs alone leaves no SoC evidence — and
# writing an unverified rootfs for a foreign SoC bricks the device, so that
# is the one case worth refusing.
if [ "1" != "$update_kernel" ] && [ "1" != "$skip_soc" ]; then
losetup -d "$loop" 2>/dev/null
rm -rf "$y"
die "Cannot verify the SoC of an unmountable rootfs. Flash the matching kernel in the same run, or pass --skip_soc."

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--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.

fi
echo_c 33 "Skipping the rootfs version/SoC pre-check; SoC is validated from the kernel image."

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

fi

[ -n "$loop" ] && losetup -d "$loop" 2>/dev/null
rm -rf "$y"
return 0
}

do_update_rootfs() {
local x=$1
[ -z "$x" ] && x="/tmp/rootfs.squashfs.$model"
echo_c 33 "\nRootFS"
echo "Update rootfs from $x"
[ ! -f "$x" ] && die "File $x not found"
[ "1" = "$exit_update" ] && return 0
set_progress flashcp -v "$x" "$(get_device "rootfs")"
echo_c 32 "RootFS updated to $rootfs_version"
echo_c 32 "RootFS updated to ${rootfs_version:-unknown}"
}

do_update_firmware() {
Expand Down Expand Up @@ -131,6 +183,7 @@ do_update_firmware() {
echo "Split combined image: FIT ${fitsz}B -> kernel, remainder -> rootfs"
dd if="$x" bs=65536 count="$blocks" of=/tmp/uImage.$model 2>/dev/null
dd if="$x" bs=65536 skip="$blocks" of=/tmp/rootfs.squashfs.$model 2>/dev/null
verify_rootfs "/tmp/rootfs.squashfs.$model"

@widgetii widgetii Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

do_update_kernel "/tmp/uImage.$model"
do_update_rootfs "/tmp/rootfs.squashfs.$model"
return 0
Expand Down Expand Up @@ -591,6 +644,12 @@ if [ "1" = "$image_combined" ]; then
# maps to a single combined write.
{ [ "1" = "$update_kernel" ] || [ "1" = "$update_rootfs" ]; } && do_update_firmware
else
# Verify the rootfs BEFORE the first write. The kernel is flashed first, and
# this check used to live inside do_update_rootfs — so a rootfs that failed
# verification was only ever discovered once the kernel had already been
# committed, leaving a half-upgraded device: new kernel, old rootfs, and no
# way back except a manual flash. Checking first means a failure costs nothing.
[ "1" = "$update_rootfs" ] && verify_rootfs "$rootfs_file"
[ "1" = "$update_kernel" ] && do_update_kernel "$kernel_file"
[ "1" = "$update_rootfs" ] && do_update_rootfs "$rootfs_file"
fi
Expand Down
Loading