[widgetii]

Follow-up to the finding on #52 ("Damaged kernels halt without recovery").

The scan clears bootfail once it finds the split layout, which it judges from the uImage's magic and where the squashfs sits. A kernel with a sound header but a damaged payload then fails bootm (Bad Data CRC, reset), and nothing asks for recovery. fwrecovery sees no bootfail, and the boot-limit fallback boots the same image again.

The split branch of firmware_scan now reads the kernel back from the flash and checks its header and data CRC (nor_split_kernel_ok). nor_split_layout only trusts the size from a header that passes its CRC, and a uImage whose header fails it gets bootfail in either layout, because bootm refuses it regardless of verify. On a mismatch it sets bootfail and keeps everything else the scan set for the split layout. bootflash's fwrecovery then fetches firmware.bin.<soc> and writes it where the firmware starts. Without the memory for the check, the kernel is taken as it is, and bootm still checks it. This is shared code in common/openipc.c, so it covers every board with CONFIG_OPENIPC_NOR_SPLIT. Fixed slots are unchanged.

Cost: the scan reads the uImage (about 1.9 MiB on today's split images) once more at boot. That was not measured on hardware.

Tested in qemu-hisilicon (gk7205v500, 8 MB, split lite firmware.bin):

Case	Result
intact	scan passes, bootm verifies OK, root on 31:4, login
one byte flipped 1 MiB into the kernel payload	Firmware is absent/corrupt (kernel at 0x50000 fails its CRC), fwupd fetches firmware.bin.gk7205v500 over TFTP, erases 0x50000 to the end and writes it (byte-identical on flash), next boot reaches login on 31:4
ih_size corrupted in the kernel header	kernel header at 0x50000 fails its CRC, the same recovery, next boot reaches login on 31:4
same damaged image, current U-Boot	Bad Data CRC, can't get kernel image!, reset, with no recovery
fixed slots	unchanged: cmdlinepart, root on 31:3

Builds for gk7205v500, hi3516cv300, hi3516ev300 and hi3516cv500 (no split support) with no new warnings. Not run on hardware.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Recover when a split NOR kernel fails its CRC

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Validate split-layout kernel header and payload CRCs during firmware scanning.
• Mark corrupt kernels for firmware recovery instead of repeatedly booting the same image.
• Preserve split-layout settings; leave fixed-slot scanning unchanged.

Diagram

graph TD
  F["SPI NOR firmware"] --> S["Split-layout scan"] --> C{"Kernel CRC valid?"} -->|no| B["Set bootfail"] --> R["Firmware recovery"]
  C -->|yes or no memory| N["Normal boot"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Recover after bootm fails

• ➕ Avoids rereading the kernel during every split-layout scan.
• ➖ Depends on bootm returning control after CRC failure; the reported failure resets instead.
• ➖ Requires recovery handling in the boot command and boot-limit paths.

Recommendation: Keep the scan-time check: it sets bootfail before attempting a bootm that may reset, and it reuses the established recovery path. The extra flash read is the main trade-off and has not been timed on hardware.

Files changed (1) +46 / -1

Bug fix (1) +46 / -1

openipc.cCheck split NOR kernel CRCs before clearing bootfail +46/-1

Check split NOR kernel CRCs before clearing bootfail

• Adds a bounded, chunked flash read to verify the uImage header and payload CRC for detected split layouts. The scan retains its layout configuration but sets bootfail on verification failure, allowing existing firmware recovery to run; allocation failure leaves verification to bootm.

common/openipc.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Kernel checks can corrupt boot memory ✓ Resolved

Description

nor_split_kernel_ok() passes a buffer from ordinary malloc() to an FMC DMA read, whose cache
invalidation rounds outward to cache-line boundaries. On split-NOR boards using that controller with
data cache enabled, the rounded range can include neighboring heap metadata or data, so discarding a
dirty cache line can disrupt later allocations.

Code

common/openipc.c[726]

+  buf = malloc(chunk);

Evidence

The FMC read invalidates cache lines around the supplied buffer, while the existing flash-read code
deliberately allocates a cache-aligned buffer to avoid affecting neighboring allocations.

common/openipc.c[726-735]
drivers/mtd/spi/fmc100/fmc100.c[327-336]
common/openipc.c[1015-1023]
include/memalign.h[101-114]
openipc/hi3516cv608_config[55-55]

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 new CRC read buffer may share cache lines with heap metadata, which the FMC DMA driver invalidates during reads.
## Fix Focus Areas
- common/openipc.c[726-735]
## Recommended Fix
Allocate the buffer with `malloc_cache_aligned(chunk)`, as the existing flash scanner does, and retain the existing `free()`.

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

---

2. Damaged headers can bypass recovery ✓ Resolved

Description

firmware_scan() calls nor_split_kernel_ok() only after nor_split_layout() matches a rootfs
position calculated from the uImage's unchecked size field. If corruption changes that field while
leaving the magic intact, the scanner can still find the actual rootfs, but the layout comparison
fails and the ordinary scan path clears bootfail without checking the header CRC.

Code

common/openipc.c[864]

+    int kernel_ok = nor_split_kernel_ok(flash, kernel_off);

Evidence

Discovery accepts uImage magic without a header CRC check; split detection then uses that header's
size, and only the matched split branch invokes the new validator. The fallback clears the failure
flag when both offsets were found.

common/openipc.c[82-94]
common/openipc.c[169-193]
common/openipc.c[682-698]
common/openipc.c[860-864]
common/openipc.c[972-983]

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 corrupted uImage size field can prevent the new CRC check from running even when the split kernel and rootfs are found.
## Fix Focus Areas
- common/openipc.c[682-698]
- common/openipc.c[860-864]
- common/openipc.c[969-983]
## Recommended Fix
Do not treat an unchecked header-size mismatch as proof that a found image is healthy. Validate the candidate header before using its size to select the healthy split path, and preserve `bootfail` when an invalid header belongs to a detected split firmware candidate.

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

---

3. Failed recovery repeatedly resets devices 🐞 Bug ☼ Reliability

Description

The new CRC-failure branch sets bootfail, which makes fwrecovery run an update and then reset
without checking whether the update succeeded. When no recovery file is available or a write fails,
the damaged kernel remains, the next scan sets bootfail again, and the device repeats the update
attempt and reset instead of stopping at a prompt.

Code

common/openipc.c[R910-912]

+    } else {
+      printf("Firmware is absent/corrupt (kernel at 0x%lx fails its CRC)\n", kernel_off);
+      env_set("bootfail", "1");

Evidence

The new assignment activates the existing unconditional reset in fwrecovery; the split update
command can fail to fetch or write its files, and the scan repeats after a reset without persisting
a successful repair.

common/openipc.c[908-914]
include/configs/openipc-common.h[180-184]
include/configs/openipc-common.h[203-217]
include/configs/openipc-common.h[230-244]

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 failures now enter a recovery command that resets even when no valid update was installed, producing an endless recovery cycle.
## Fix Focus Areas
- common/openipc.c[908-913]
- include/configs/openipc-common.h[180-184]
- include/configs/openipc-common.h[203-217]
## Recommended Fix
Make split recovery distinguish a successfully written firmware image from unavailable or failed updates. Reset only after success; on failure, stop at a recoverable prompt rather than repeating the same flash operation indefinitely.

ⓘ 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 reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗