Skip to content

sysupgrade: ignore --no_reboot when the live flash was rewritten (#2231) #2236

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 1 commit into from
Jul 28, 2026
Merged

sysupgrade: ignore --no_reboot when the live flash was rewritten (#2231) #2236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
303 changes: 301 additions & 2 deletions .github/scripts/test_sysupgrade.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ sed -e 's|grep "GITHUB_VERSION" "$1/etc/os-release"|grep "GITHUB_VERSION" "${1:-
-e "s|/tmp\\b|@SB@/tmp|g" \
-e "s|/etc/os-release|@SB@/etc/os-release|g" \
-e "s|/proc/mtd|@SB@/proc/mtd|g" \
-e "s|/proc/cmdline|@SB@/proc/cmdline|g" \
-e "s|/proc/sys/vm/drop_caches|@SB@/tmp/drop_caches|g" \
-e "s|/etc/init.d/|@SB@/etc/init.d/|g" \
-e "s|@OSRELEASE@|/etc/os-release|g" \
Expand All @@ -74,7 +75,9 @@ for s in S99rc.local S60crond S49ntpd S02klogd S01syslogd; do
printf '#!/bin/sh\nexit 0\n' > "$SB/etc/init.d/$s"; chmod +x "$SB/etc/init.d/$s"
done

cat > "$SB/proc/mtd" <<'EOF'
set_mtd() { cat > "$SB/proc/mtd"; }

set_mtd <<'EOF'
dev: size erasesize name
mtd0: 00040000 00010000 "boot"
mtd1: 00010000 00010000 "env"
Expand All @@ -83,6 +86,17 @@ mtd3: 00500000 00010000 "rootfs"
mtd4: 00100000 00010000 "rootfs_data"
EOF

# The kernel command line decides whether this camera is running FROM the flash
# sysupgrade is about to rewrite. The default is the real one off the lab
# hi3516ev300 -- deliberately including mmz=anonymous and an mtdparts label
# ending in "rootfs_data", the substrings a loose root-type probe trips over.
set_cmdline() { printf '%s\n' "$1" > "$SB/proc/cmdline"; }
CMDLINE_FLASH='mem=128M console=ttyAMA0,115200 panic=20 rootfstype=squashfs root=/dev/mtdblock3 mtdparts=hi_sfc:256k(boot),64k(env),2048k(kernel),5120k(rootfs),-(rootfs_data) mmz_allocator=cma mmz=anonymous,0,0x42000000,96M init=/init'
CMDLINE_NFS='console=ttyAMA0,115200 root=/dev/nfs nfsroot=192.168.1.1:/srv/cam,tcp,v3 ip=dhcp rw'
CMDLINE_MMC='console=ttyAMA0,115200 root=/dev/mmcblk0p2 rootfstype=ext4 rw'
CMDLINE_RAM='console=ttyAMA0,115200 root=/dev/ram0 rdinit=/linuxrc'
set_cmdline "$CMDLINE_FLASH"

# --- stubs -----------------------------------------------------------------
stub() { printf '#!/bin/bash\n%s\n' "$2" > "$SB/bin/$1"; chmod +x "$SB/bin/$1"; }

Expand Down Expand Up @@ -112,10 +126,14 @@ chmod +x "$SB/bin/md5sum"
# flashcp / flash_eraseall / reboot all arrive via `busybox <applet>`; record the
# argv of anything that writes, in order, so a test can assert both WHAT was
# written and WHETHER anything was.
#
# STUB_FLASHCP_FAIL makes the write fail AFTER it has been logged -- a partially
# erased partition, which is the state do_update_firmware's `|| die` reacts to.
stub busybox '
applet=$1; shift
case "$applet" in
flashcp|flash_eraseall) echo "$applet $*" >> "$FLASH_LOG" ;;
flashcp|flash_eraseall) echo "$applet $*" >> "$FLASH_LOG"
[ "1" = "$STUB_FLASHCP_FAIL" ] && exit 1 ;;
reboot) echo "reboot" >> "$FLASH_LOG"; exit 0 ;;
esac
exit 0'
Expand Down Expand Up @@ -203,23 +221,39 @@ run() {
: > "$SB/tmp/flash.log"
OUT=$(cd "$SB" && env PATH="$SB/bin:$PATH" \
HASERLVER=1 FLASH_LOG="$SB/tmp/flash.log" mount_wait="${MOUNT_WAIT:-3}" \
abort_wait=0 \
STUB_MOUNT="${STUB_MOUNT:-ok}" STUB_VENDOR="${STUB_VENDOR:-sigmastar}" \
STUB_SOC="${STUB_SOC:-ssc338q}" \
STUB_IMG_SOC="${STUB_IMG_SOC:-ssc338q}" \
STUB_IMG_VERSION="${STUB_IMG_VERSION:-2026.07.11}" \
STUB_FLASHCP_FAIL="${STUB_FLASHCP_FAIL:-0}" \
sh "$SB/sysupgrade" "$@" 2>&1)
RC=$?
}
flashed() { grep -q "flashcp .*$1" "$SB/tmp/flash.log"; }
erased() { grep -q "flash_eraseall .*$1" "$SB/tmp/flash.log"; }
nothing_wrote() { ! grep -q "flashcp" "$SB/tmp/flash.log"; }
# The busybox stub logs a bare "reboot" line, so whether the run rebooted is
# directly observable -- which is the whole question issue #2231 turns on.
rebooted() { grep -qx "reboot" "$SB/tmp/flash.log"; }
# Line number of a phrase in $OUT, for ordering assertions.
at() { printf '%s\n' "$OUT" | grep -n -- "$1" | head -1 | cut -d: -f1; }

reset_env() {
unset STUB_MOUNT STUB_VENDOR STUB_SOC STUB_IMG_SOC STUB_IMG_VERSION MOUNT_WAIT
unset STUB_FLASHCP_FAIL
rm -f "$SB"/tmp/*.ssc338q "$SB"/tmp/firmware.bin.* "$SB"/tmp/*.tgz "$SB"/tmp/*.md5sum
make_uimage "$SB/tmp/uImage.ssc338q" ssc338q
make_rootfs "$SB/tmp/rootfs.squashfs.ssc338q"
set_cmdline "$CMDLINE_FLASH"
set_mtd <<'EOF'
dev: size erasesize name
mtd0: 00040000 00010000 "boot"
mtd1: 00010000 00010000 "env"
mtd2: 00200000 00010000 "kernel"
mtd3: 00500000 00010000 "rootfs"
mtd4: 00100000 00010000 "rootfs_data"
EOF
}

K="$SB/tmp/uImage.ssc338q"
Expand Down Expand Up @@ -398,6 +432,198 @@ else
bad "verify should print its own header/line before mounting"
fi

# --- -x / --no_reboot on a camera flashing its own live rootfs (issue #2231) --
#
# A NOR camera boots an overlay whose lowerdir is the squashfs on the "rootfs"
# MTD partition and whose upperdir is the jffs2 on "rootfs_data" -- the two
# partitions sysupgrade erases. free_resources() drops the page cache to make
# room for the download, so once flashcp has rewritten that partition every
# read that misses the cache comes back as the NEW image at STALE offsets: SSH
# auth, libc and /etc all return garbage. Honouring --no_reboot there does not
# leave a working camera pending a convenient reboot, it leaves a dead one that
# still answers ping and cannot be logged into to issue the reboot at all.
#
# So the reboot decision cannot rest on skip_reboot alone. It has to ask what
# actually got written, and whether this camera is running from it.

echo
echo "=== Part 1b: --no_reboot vs the live rootfs (issue #2231) ==="

# The bug itself. -x must not be honoured once the live rootfs is overwritten.
reset_env
run -z --rootfs="$R" -x
if [ "$RC" -eq 0 ] && flashed /dev/mtd3 && rebooted; then
ok "-x + live rootfs rewritten -> reboots anyway"
else
bad "-x + live rootfs -> expected flash then reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi
# And it must say why, rather than rebooting a camera whose operator asked it not to.
if printf '%s' "$OUT" | grep -q -- "--no_reboot ignored"; then
ok "-x override explains itself"
else
bad "-x override must explain itself; got: $(printf '%s' "$OUT" | tail -3)"
fi
# The warning has to come BEFORE the write, while Ctrl-C still works: the trap
# that shields flashcp from a dying TTY also takes the operator's way out.
n=$(at "NOTICE"); w=$(at "Update rootfs from")
if [ -n "$n" ] && [ -n "$w" ] && [ "$n" -lt "$w" ]; then
ok "-x notice precedes the first write (abort window is real)"
else
bad "-x notice must precede the write -- notice@${n:-none} write@${w:-none}"
fi

# The flag still has a job. A kernel-only run never touches the mounted
# partition, so the camera survives it and -x means what it says.
reset_env
run -z --kernel="$K" -x
if [ "$RC" -eq 0 ] && flashed /dev/mtd2 && ! flashed /dev/mtd3 && ! rebooted; then
ok "-x + kernel only -> honoured, no reboot"
else
bad "-x + kernel only -> expected no reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi
# ...and exits 0. A deliberate --no_reboot used to report failure.
if printf '%s' "$OUT" | grep -q "asked me not to reboot"; then
ok "-x honoured path prints the soft notice and exits $RC"
else
bad "-x honoured path should print the soft notice; got: $(printf '%s' "$OUT" | tail -3)"
fi

# Same version: do_update_rootfs returns before flashcp, nothing is written, so
# there is nothing to reboot for. Deciding on intent rather than on what was
# written would get this wrong.
reset_env
STUB_IMG_VERSION=2026.06.01
run -z --rootfs="$R" -x
if [ "$RC" -eq 0 ] && ! flashed /dev/mtd3 && ! rebooted; then
ok "-x + same version -> nothing written, honoured"
else
bad "-x + same version -> expected no write and no reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# Not every camera runs from the flash it writes. An NFS/SD/ram root is a
# supported layout (general/overlay/init, general/package/openipc-nfs-root)
# where the rootfs partition is just a target and -x is exactly the right flag.
for c in "$CMDLINE_NFS:nfs" "$CMDLINE_MMC:mmcblk" "$CMDLINE_RAM:ram"; do
reset_env
set_cmdline "${c%:*}"
run -z --rootfs="$R" -x
if [ "$RC" -eq 0 ] && flashed /dev/mtd3 && ! rebooted; then
ok "-x + ${c##*:} root -> rootfs flashed, honoured (not running from it)"
else
bad "-x + ${c##*:} root -> expected flash with no reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi
done

# The failure that would brick a camera is a FALSE "not on flash", so the probe
# is anchored to the root= token. The stock hi3516ev300 command line carries
# both `mmz=anonymous` and an mtdparts label ending `-(rootfs_data)`; a probe
# matching a bare `ram`/`mmcblk` anywhere in the line is one vendor bootarg away
# from waving through the exact case this test exists for.
reset_env
set_cmdline "$CMDLINE_FLASH ramdisk_size=8192 mmz=mmcblkish"
run -z --rootfs="$R" -x
if flashed /dev/mtd3 && rebooted; then
ok "-x + decoy 'ram'/'mmcblk' substrings -> still recognised as flash root"
else
bad "root-type probe must anchor to root=; decoy substrings fooled it, log='$(cat "$SB/tmp/flash.log")'"
fi

# The combined image (cv6xx) reaches the rootfs by both of its paths.
reset_env
make_combined "$SB/tmp/firmware.bin.ssc338q"
make_archive "$SB/tmp/firmware.bin.ssc338q"
run -z --archive="$SB/tmp/fw.tgz" -x
if flashed /dev/mtd2 && flashed /dev/mtd3 && rebooted; then
ok "-x + combined image, split path -> reboots anyway"
else
bad "-x + combined split -> expected both flashed and a reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# No separate kernel/rootfs partitions: one whole-blob write to "firmware",
# which overlaps the running rootfs.
reset_env
set_mtd <<'EOF'
dev: size erasesize name
mtd0: 00040000 00010000 "boot"
mtd1: 00010000 00010000 "env"
mtd2: 00700000 00010000 "firmware"
EOF
make_combined "$SB/tmp/firmware.bin.ssc338q"
make_archive "$SB/tmp/firmware.bin.ssc338q"
run -z --archive="$SB/tmp/fw.tgz" -x
if flashed /dev/mtd2 && rebooted; then
ok "-x + combined image, whole-blob path -> reboots anyway"
else
bad "-x + combined whole-blob -> expected flash and reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# A write that FAILS is not a write that did not happen: flashcp erases before
# it writes, so a partition left half-erased is every bit as unreadable. This
# is why the run is marked dirty before the write, not after -- do_update_firmware
# reaches reboot_system through `|| die` precisely here.
reset_env
set_mtd <<'EOF'
dev: size erasesize name
mtd0: 00040000 00010000 "boot"
mtd1: 00010000 00010000 "env"
mtd2: 00700000 00010000 "firmware"
EOF
make_combined "$SB/tmp/firmware.bin.ssc338q"
make_archive "$SB/tmp/firmware.bin.ssc338q"
STUB_FLASHCP_FAIL=1
run -z --archive="$SB/tmp/fw.tgz" -x
if [ "$RC" -ne 0 ] && rebooted; then
ok "-x + write started then failed -> reboots anyway (partial erase is fatal too)"
else
bad "-x + failed write -> expected a reboot despite the failure, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# --wipe_overlay erases "rootfs_data" -- the rw jffs2 that is the upperdir of
# the running overlay, so overlayfs consults it for every lookup. Erasing it
# live breaks the camera just as thoroughly as rewriting the lowerdir.
reset_env
run -z --wipe_overlay -x
if erased /dev/mtd4 && rebooted; then
ok "-x + --wipe_overlay on flash root -> reboots anyway (live upperdir erased)"
else
bad "-x + --wipe_overlay -> expected erase and reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi
# On a non-flash root init mounts a tmpfs overlay instead, so there is no live
# upperdir on the partition being erased.
reset_env
set_cmdline "$CMDLINE_NFS"
run -z --wipe_overlay -x
if [ "$RC" -eq 0 ] && erased /dev/mtd4 && ! rebooted; then
ok "-x + --wipe_overlay on nfs root -> honoured"
else
bad "-x + --wipe_overlay on nfs root -> expected no reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# Nothing was written, so a failure before the first flash still honours -x.
reset_env
STUB_IMG_SOC=gk7205v300
run -z --rootfs="$R" -x
if [ "$RC" -ne 0 ] && nothing_wrote && ! rebooted; then
ok "-x + refusal before any write -> honoured, nothing written"
else
bad "-x + pre-write refusal -> expected no write and no reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi

# The default path must be untouched by all of the above.
reset_env
run -z --kernel="$K" --rootfs="$R"
if [ "$RC" -eq 0 ] && flashed /dev/mtd2 && flashed /dev/mtd3 && rebooted; then
ok "no -x -> unconditional reboot, unchanged"
else
bad "no -x -> expected both flashed and a reboot, rc=$RC log='$(cat "$SB/tmp/flash.log")'"
fi
# ...and it must not print the -x notice at people who never passed -x.
if ! printf '%s' "$OUT" | grep -q "NOTICE\|--no_reboot ignored"; then
ok "no -x -> no --no_reboot chatter"
else
bad "a run without -x should not mention --no_reboot"
fi

# ---------------------------------------------------------------------------
echo
echo "=== Part 2: invariants in $SRC ==="
Expand Down Expand Up @@ -444,6 +670,79 @@ else
bad "CONFIG_TIMEOUT was dropped from busybox.config -- the bounded mount degrades"
fi

# --- issue #2231 invariants ------------------------------------------------

# skip_reboot must never be the sole gate again. The behaviour tests above only
# see the cases they were written for; this pins the shape.
rb=$(sed -n '/^reboot_system()/,/^}/p' "$SRC")
if printf '%s' "$rb" | grep -q 'live_flash_dirty'; then
ok "reboot_system weighs what was written, not just skip_reboot"
else
bad "reboot_system must consult live_flash_dirty, not skip_reboot alone"
fi

# It also must not exit on the honoured path: die() and the main flow disagree
# about the status, and the main flow's `exit 0` is what makes -x a success.
# Comments are stripped -- the ones in there discuss exit codes at length.
rbc=$(printf '%s\n' "$rb" | sed 's/#.*//')
if printf '%s\n' "$rbc" | grep -q 'return 0' && ! printf '%s\n' "$rbc" | grep -qw 'exit'; then
ok "reboot_system returns rather than exits (caller owns the status)"
else
bad "reboot_system must return on the honoured path so die() keeps its own exit 1"
fi

# Every write that lands on flash the camera is running from has to be marked,
# and the one that does not must stay unmarked or -x loses its only real use.
for fn in do_update_rootfs do_update_firmware do_wipe_overlay; do
if sed -n "/^${fn}()/,/^}/p" "$SRC" | grep -q 'mark_live_flash_dirty'; then
ok "$fn marks the live flash dirty"
else
bad "$fn writes flash the camera runs from and must mark it dirty"
fi
done
if sed -n '/^do_update_kernel()/,/^}/p' "$SRC" | grep -q 'mark_live_flash_dirty'; then
bad "do_update_kernel must NOT mark dirty -- the kernel partition is not mounted"
else
ok "do_update_kernel leaves -x alone (its partition is not mounted)"
fi

# The mark belongs before the write (a half-erased partition is just as dead)
# and after the same-version return (which writes nothing at all).
body=$(sed -n '/^do_update_rootfs()/,/^}/p' "$SRC")
e=$(printf '%s\n' "$body" | grep -n 'exit_update' | head -1 | cut -d: -f1)
m=$(printf '%s\n' "$body" | grep -n 'mark_live_flash_dirty' | head -1 | cut -d: -f1)
f=$(printf '%s\n' "$body" | grep -n 'flashcp' | head -1 | cut -d: -f1)
if [ -n "$e" ] && [ -n "$m" ] && [ -n "$f" ] && [ "$e" -lt "$m" ] && [ "$m" -lt "$f" ]; then
ok "do_update_rootfs marks after the same-version return and before the write"
else
bad "do_update_rootfs order must be exit_update -> mark -> flashcp; got ${e:-none}/${m:-none}/${f:-none}"
fi

# The root-type probe must anchor to root=. An unanchored alternation matches a
# bare 'ram'/'mmcblk' anywhere in the command line, and a false "not on flash"
# is the one error here that bricks a camera.
if grep -q 'root_on_flash' "$SRC" \
&& grep -qF 'root=(/dev/)?(nfs|mmcblk|ram)' "$SRC" \
&& ! grep -qF 'nfs\|mmcblk\|ram' "$SRC"; then
ok "root-type probe is anchored to the root= token"
else
bad "root-type probe must anchor each alternative to root=, not match bare substrings"
fi

# The abort window has to stay overridable, or this suite pays for it six times.
if grep -q 'abort_wait=${abort_wait:-' "$SRC"; then
ok "the -x abort window is overridable (abort_wait)"
else
bad "abort_wait must stay overridable so the suite does not sleep through it"
fi

# The caveat belongs in --help, not only in the code.
if sed -n '/-x, --no_reboot/,/-z, --no_update/p' "$SRC" | grep -qi 'ignored'; then
ok "--help says -x is ignored when the live flash is rewritten"
else
bad "-x usage text must document that it is ignored on a live-flash rewrite"
fi

echo
if [ "$fail" -eq 0 ]; then
echo "All sysupgrade verification checks passed."
Expand Down
Loading
Loading