nand-migrate: move a NAND camera off the kernel-volume layout without UART by widgetii · Pull Request #243 · OpenIPC/ipctool

widgetii · 2026-10-12T18:55:39Z

Why

The first FIT NAND layout (u-boot-xmedia e19b798a, firmware #2526) kept the kernel in a UBI volume of its own: kernel (FIT), rootfs (UBIFS), rootfs_data. The current one (u-boot-xmedia a138a692, firmware #2537) has no kernel volume. The kernel is /boot/fitImage inside the UBIFS rootfs. Firmware's sysupgrade refuses the old layout and asks for a reinstall from U-Boot, which needs a serial console.

No UART is needed. A camera that saved the old layout's stock bootcmd has a FIT NAND build of u-boot-xmedia, and every NAND build has CMD_UBIFS. So its U-Boot already boots the new layout, and only the flash contents and bootcmd have to change.

What

ipctool nand-migrate [--dry-run] [--wipe-settings] <rootfs.ubifs>, where the image is rootfs.ubifs.<soc> from the -nand- package to install.

It runs these steps in order, so a run stopped at any point after step 1 still boots:

write the image into the rootfs volume, and compare it read back (SHA-1);
set bootcmd to the current stock one (ubifsload /boot/fitImage, then /boot/uImage);
remove the kernel volume, then reboot.

A stop after step 1 boots the old kernel volume with the new rootfs. A stop after step 2 boots /boot. A camera left with the new bootcmd and a kernel volume is finished live, with no reboot. U-Boot is never written.

Step 1 rewrites the volume the system runs from, so the work runs as PID 1 from a RAM root. ipctool pivots into a tmpfs (moving /tmp along when the image is there, so it isn't copied) and sends busybox init SIGQUIT. Firmware's inittab ::restart:/sbin/init then execs the static ipctool. The log is left in /root/nand-migrate.log.

Before anything changes it refuses, saying why, when:

the layout isn't the kernel-volume one;
the kernel volume isn't a FIT;
bootcmd isn't the stock one (an edited one is left to its owner);
bootargs lacks ${rootargs};
the image isn't UBIFS for this LEB size, or has no kernel in /boot (found from its directory-entry nodes);
the image is bigger than the volume;
there isn't enough RAM;
the env's first block is bad;
ipctool is a dynamic build;
the inittab has no restart action.

The env is located, edited (exact key match) and CRC'd in nand_layout.c, which nand_layout_test covers. It is written with every erase block it spans erased first. The existing mtd_write() erases one block, which is too little for an env larger than a NAND block.

Tested

nand_layout_test covers synthetic env and UBIFS buffers, plus (via NM_ENV/NM_IMAGE) this camera's real env dump and the builder image. It's added to pr-build-check.

QEMU (qemu-hisilicon gk7205v510, with the NAND erase fix 090aaec), on a replica of the camera's flash (its own U-Boot, env and three UBI volumes):

full run, then a cold boot: Loading file '/boot/fitImage', Verifying Hash Integrity ... crc32+ sha1+ OK, login;
IPCTOOL_MIGRATE_STOP=write, then a cold boot: boots via the kernel volume with the new rootfs; a re-run is accepted;
IPCTOOL_MIGRATE_STOP=env, then a cold boot: boots /boot/fitImage; the next run removes the kernel volume live;
refused, with the volumes byte-identical afterwards: a UBIFS image without /boot, a non-UBIFS file, an edited bootcmd.

Hardware: Zenointel SD-2N-4G, gk7205v510, Oct-04 xm720xxx U-Boot. The image is rootfs.ubifs.gk7205v500 from builder gk7205v510_ultimate_zenointel-sd-2n-4g-nand.tgz.

Camera: retired NAND layout, UBI device ubi0 on mtd2
  kernel ubi0_0 (FIT), rootfs ubi0_1 (32860 KB), rootfs_data ubi0_2
  bootcmd: the kernel-volume stock one; env at mtd1, 65536 bytes
Image: /tmp/rootfs.ubifs.gk7205v500, 14012 KB, UBIFS for 126976-byte LEBs, kernel /boot/fitImage
...
Handing PID 1 over to the RAM root. SSH ends here; ...

Back on SSH 1m46s later:

# cat /proc/cmdline
mem=128M console=ttyAMA0,115200 panic=20 init=/init root=ubi0:rootfs rootfstype=ubifs ubi.mtd=2,2048 mtdparts=nand:768k(boot),256k(env),-(ubi) mmz_allocator=cma mmz=anonymous,0,0x42000000,96M
# ubinfo -a | grep -E "Volumes count|Name:"
Volumes count:                           2
Name:        rootfs
Name:        rootfs_data
# fw_printenv bootcmd
bootcmd=ubi part ubi; ubifsmount ubi0:rootfs; setenv rootargs root=ubi0:rootfs rootfstype=ubifs; setenv setargs setenv bootargs ${bootargs}; run setargs; if ubifsload ${baseaddr} /boot/fitImage; then bootm ${baseaddr}; fi; if ubifsload ${baseaddr} /boot/uImage; then bootm ${baseaddr}; fi; reset
# grep -E "GITHUB_VERSION|BUILD_PLATFORM" /etc/os-release
GITHUB_VERSION="master+a40b399, 2026-10-11"
BUILD_PLATFORM=gk7205v510_ultimate_zenointel-sd-2n-4g
# uname -v
#1 Sun Oct 11 19:19:35 UTC 2026
# cat /root/nand-migrate.log   (progress lines trimmed)
running as PID 1 from RAM
writing the rootfs volume ubi0_1 (14348288 bytes)
rootfs written and read back: SHA-1 matches
bootcmd now loads the kernel from /boot in the rootfs
kernel volume removed
done, rebooting

majestic came back up with the mis2008 sensor and its VENC channels.

… UART The first FIT NAND layout kept the kernel in a UBI volume of its own; the current one keeps it as /boot/fitImage in the UBIFS rootfs, and firmware's sysupgrade refuses the old one, asking for a reinstall from U-Boot -- which needs a serial console. Every U-Boot that saved that layout's stock bootcmd is a FIT NAND build with ubifsload, so it already boots the new layout. nand-migrate writes a rootfs that carries /boot (the -nand- package's rootfs.ubifs) into the rootfs volume, switches bootcmd to the current stock one, and removes the kernel volume -- in that order, so a run stopped after the write still boots. It runs as PID 1 from a RAM root, handed over through the ::restart:/sbin/init action firmware's inittab carries, since UBI will not update the volume the system runs from. U-Boot is not written. Before anything is touched it refuses, saying why: an edited bootcmd, a kernel volume without a FIT, bootargs without ${rootargs}, an image that is not UBIFS for this LEB size or has no kernel in /boot, too little RAM, a dynamic build, an inittab without the restart action. A camera stopped after the bootcmd switch is finished live. The log is left in /root/nand-migrate.log. The U-Boot env is located, edited (exact key match) and CRC'd by nand_layout.c, and written with every erase block it spans erased first.

openipc-ai · 2026-10-12T19:28:33Z

/review

qodo-free-for-open-source-projects · 2026-10-12T19:32:22Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~A failed rootfs write can leave no bootable root~~ ✓ Resolved

Description

ubi_volup() starts an in-place update of the existing rootfs volume, and nand_migrate_pid1()
reboots immediately if that update or its readback fails. Once the update has begun, an incomplete
rootfs can be left behind; keeping the old kernel volume does not help because its boot path still
needs that same rootfs.

Code

src/nand_migrate.c[R458-461]

+    if (!ubi_volup(c.ubi, c.rootfs, img, size)) {
+        say("writing the rootfs failed: %s", strerror(errno));
+        finish(&c, false);
+    }

Evidence

The update operates on the existing rootfs device, while both the write-error and readback-error
paths call the rebooting finish function before changing bootcmd. The old bootcmd's retained kernel
volume supplies a kernel, not another rootfs.

src/nand_migrate.c[251-284]
src/nand_migrate.c[455-483]
src/nand_migrate.c[366-375]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An interrupted or failed in-place UBI update can leave the shared rootfs incomplete, even though the old kernel volume remains.
## Fix Focus Areas
- src/nand_migrate.c[249-292]
- src/nand_migrate.c[455-465]
## Recommended Fix
Provide a recoverable update path that preserves a bootable rootfs until the new image has been written and verified. If the device cannot accommodate one, do not claim that a failed step-one update can safely reboot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Long image paths can corrupt the stack~~ ✓ Resolved

Description

nand_migrate_cmd() passes a 128-byte stack buffer to realpath(), which has no argument
describing that buffer's capacity. If an image's resolved absolute path exceeds 127 bytes,
resolution writes beyond path before the migration can reject or stage it.

Code

src/nand_migrate.c[R768-769]

+    char path[128];
+    if (!realpath(argv[optind], path)) {

Evidence

The supplied output buffer is only 128 bytes, and the resolved path is subsequently used for the
image mapping and RAM-root hand-off; no path-length check precedes the realpath call.

src/nand_migrate.c[767-775]
src/nand_migrate.c[571-581]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A resolved image path longer than the fixed stack buffer overflows it during realpath resolution.
## Fix Focus Areas
- src/nand_migrate.c[767-775]
- src/nand_migrate.c[558-589]
## Recommended Fix
Call realpath with a NULL output buffer and free the allocated result on every exit path. If moving a `/tmp` image requires its path to fit the staged configuration, check that length explicitly before hand-off.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Edited boot arguments can omit the root~~ ✓ Resolved

Description

nm_check_camera() accepts bootargs whenever ${rootargs} appears as a substring, rather than as
a separate kernel argument. For example, foo=${rootargs}bar passes preflight, but the new bootcmd
expands it into foo=root=ubi0:rootfs rootfstype=ubifsbar, leaving no standalone root= argument.

Code

src/nand_layout.c[R131-133]

+    if (!cam->bootargs || !strstr(cam->bootargs, "${rootargs}"))
+        REFUSE("bootargs does not take its root from ${rootargs}, which the "
+               "new bootcmd sets");

Evidence

The check uses strstr even though this file has a word-boundary helper. The replacement bootcmd
expands rootargs through the existing bootargs value, so embedding the placeholder changes the
resulting kernel arguments.

src/nand_layout.c[91-98]
src/nand_layout.c[127-134]
src/nand_layout.h[33-38]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A substring match permits boot arguments whose expansion does not supply a standalone root device.
## Fix Focus Areas
- src/nand_layout.c[91-98]
- src/nand_layout.c[131-134]
## Recommended Fix
Validate `${rootargs}` as a complete space-delimited argument, and add a test with the placeholder embedded in another argument.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Deleted kernel entries can pass image checks~~ ✓ Resolved

Description

nm_check_image() treats any CRC-valid /boot and kernel directory entries it finds as current,
but skips deletion entries without invalidating an earlier match. An image containing an older
fitImage entry and a later deletion can therefore pass preflight even though /boot/fitImage is
absent from the filesystem the new bootcmd reads.

Code

src/nand_layout.c[R186-189]

+            if (!inum || 56u + nlen > len)
+                continue;
+            if (pass == 0 && parent == UBIFS_ROOT_INO &&
+                type == UBIFS_ITYPE_DIR && nlen == 4 &&

Evidence
The scanner searches raw nodes, ignores zero-inode deletion entries, and never clears a previously
recorded match. Its result gates installation before the rootfs update and bootcmd switch.
src/nand_layout.c[167-205]
src/nand_migrate.c[779-789]
src/nand_migrate.c[455-516]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Scanning directory-entry nodes without resolving later deletions can mistake a stale kernel entry for a present file.
## Fix Focus Areas
- src/nand_layout.c[167-205]
- src/nand_layout_test.c[188-205]
## Recommended Fix
Validate the effective UBIFS directory state rather than the existence of historical entries, or restrict accepted inputs to a format whose directory entries cannot contain stale versions. Test an entry followed by its deletion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Malformed environment values can read past memory~~ ✓ Resolved

Description

nm_env_get() can return a matching key=value entry even when its value has no terminating NUL
within the environment buffer. A CRC-valid but malformed environment with an unterminated bootcmd
then reaches strcmp() in nm_check_camera(), which reads beyond the allocated partition buffer.

Code

src/nand_layout.c[R47-50]

+        size_t n = strnlen(p, end - p);
+        if (n > klen && !strncmp(p, key, klen) && p[klen] == '=')
+            return p + klen + 1;
+        p += n + 1;

Evidence

The CRC check validates bytes, not string termination; nm_env_get bounds strnlen but neither rejects
an unterminated matching entry nor safely handles its end-of-buffer advance. The migration passes
its returned bootcmd to string comparisons.

src/nand_layout.c[31-53]
src/nand_layout.c[117-125]
src/nand_migrate.c[702-727]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
CRC validation alone does not ensure environment strings terminate inside the buffer, so a returned value can cause an out-of-bounds string read.
## Fix Focus Areas
- src/nand_layout.c[31-53]
- src/nand_migrate.c[702-727]
## Recommended Fix
Reject an entry when bounded strnlen reaches the remaining buffer length, before returning its value or advancing the pointer. Add a CRC-valid unterminated-entry test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View medium (1)

6. ~~Cross-block entries can pass image checks~~ ✓ Resolved

Description

nm_check_image() gives ubifs_node() the remaining length of the entire image rather than the
space remaining in the current logical erase block. A CRC-valid directory entry spanning two blocks
can consequently count as /boot or its kernel even though the node is not contained within a block
as the scanner's own layout rule requires.

Code

src/nand_layout.c[R175-177]

+        for (size_t off = 0; off + UBIFS_CH_SZ <= size; off += 8) {
+            const uint8_t *p = img + off;
+            if (p[20] != UBIFS_DENT_NODE || !(len = ubifs_node(p, size - off)))

Evidence

The code states that nodes are within LEBs, yet passes size minus offset to the node validator and
checks node length only against that value. A crossing node therefore meets the implemented length
condition and can influence the kernel-name result used by preflight.

src/nand_layout.c[145-155]
src/nand_layout.c[167-177]
src/nand_migrate.c[779-789]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The image scanner can recognize a fabricated directory node whose length crosses a logical erase-block boundary.
## Fix Focus Areas
- src/nand_layout.c[145-155]
- src/nand_layout.c[167-200]
## Recommended Fix
Limit each node's available room to the lesser of the image remainder and the current LEB remainder. Add a test placing a CRC-valid directory node across a LEB boundary.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

- A failed rootfs write is not survivable: once UBI begins the update the old rootfs is gone, and the kernel volume kept for step 2 boots nothing without it. A write-then-swap needs the image's size free, which this layout never has (rootfs_data is autoresize). Retry from the copy in RAM instead -- a new update starts the volume over -- and say plainly what a run that cannot write it leaves behind. - realpath() into a fixed buffer could overflow it; let it allocate, and refuse a path the one-line hand-off conf cannot carry. - ${rootargs} must be an argument of its own in bootargs. - /boot and the kernel are looked up as the filesystem has them now: the newest directory entry per name wins, so a later deletion counts. - A UBIFS node is bounded by its LEB, not by the rest of the image. - An env entry that does not end inside the env is rejected, not read past.

widgetii mentioned this pull request Oct 12, 2026

sysupgrade: point a kernel-volume NAND camera at ipctool nand-migrate OpenIPC/firmware#2578

Draft

7 tasks

qodo-free-for-open-source-projects Bot reviewed Oct 12, 2026

View reviewed changes

Comment thread src/nand_migrate.c Outdated

Comment thread src/nand_migrate.c Outdated

Comment thread src/nand_layout.c Outdated

Comment thread src/nand_layout.c Outdated

Comment thread src/nand_layout.c

Comment thread src/nand_layout.c Outdated

widgetii merged commit 9bb8b88 into master Oct 12, 2026
5 checks passed

widgetii deleted the nand-migrate branch October 12, 2026 19:59

Navigation Menu

Repository navigation

nand-migrate: move a NAND camera off the kernel-volume layout without UART - #243

nand-migrate: move a NAND camera off the kernel-volume layout without UART#243
widgetii merged 2 commits into
masterfrom
nand-migrate

widgetii commented Oct 12, 2026

Uh oh!

openipc-ai commented Oct 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

widgetii commented Oct 12, 2026

Why

What

Tested

Uh oh!

openipc-ai commented Oct 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 •

edited

Loading