Repository navigation
openipc: a split kernel that fails its CRC goes to recovery - #56
Merged
Merged
Conversation
The scan clears bootfail once it finds the split layout, from the
uImage's magic and where the squashfs sits. A kernel whose header is
sound but whose payload is damaged 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 camera loops.
The split branch of firmware_scan now reads the kernel back from the
flash and checks its header and data CRC. On a mismatch it sets bootfail,
keeping everything else the scan set for the split layout, so
bootflash's fwrecovery fetches the firmware file 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, so it covers
every board with CONFIG_OPENIPC_NOR_SPLIT. Fixed slots are unchanged.
The check reads the uImage once more at boot, about 1.9 MiB on today's
split images.
Tested in qemu-hisilicon (gk7205v500, 8 MB, split lite firmware.bin):
- intact image: the scan passes it, bootm verifies it, 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), and the next boot reaches login
on 31:4;
- the same damaged image under the 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 and hi3516ev300 with no new warnings.
Not run on hardware.
PR Summary by QodoRecover when a split NOR kernel fails its CRC
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
… buffer From review: - The check's read buffer is malloc_cache_aligned, as the scanner's is: the FMC driver reads by DMA and invalidates whole cache lines around the buffer. - nor_split_layout takes the uImage's size only from a header that passes its CRC. A damaged size field no longer makes a split image look like fixed slots, which cleared bootfail. A uImage whose header fails its CRC boots in neither layout (bootm refuses it whatever verify says), so the scan now sets bootfail for it and recovery takes over. Tested in qemu-hisilicon (gk7205v500, 8 MB): ih_size corrupted in the split kernel's header gives "Firmware is absent/corrupt (kernel header at 0x50000 fails its CRC)", then recovery over TFTP rewrites firmware.bin (byte-identical) and the next boot reaches login on 31:4. An intact split image and fixed slots boot as before. Builds for gk7205v500, hi3516cv300 and hi3516cv500 (no split support) with no new warnings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the finding on #52 ("Damaged kernels halt without recovery").
The scan clears
bootfailonce 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 failsbootm(Bad Data CRC, reset), and nothing asks for recovery.fwrecoverysees nobootfail, and the boot-limit fallback boots the same image again.The split branch of
firmware_scannow reads the kernel back from the flash and checks its header and data CRC (nor_split_kernel_ok).nor_split_layoutonly trusts the size from a header that passes its CRC, and a uImage whose header fails it getsbootfailin either layout, becausebootmrefuses it regardless ofverify. On a mismatch it setsbootfailand keeps everything else the scan set for the split layout.bootflash'sfwrecoverythen fetchesfirmware.bin.<soc>and writes it where the firmware starts. Without the memory for the check, the kernel is taken as it is, andbootmstill checks it. This is shared code incommon/openipc.c, so it covers every board withCONFIG_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):bootmverifies OK, root on 31:4, loginFirmware is absent/corrupt (kernel at 0x50000 fails its CRC),fwupdfetchesfirmware.bin.gk7205v500over TFTP, erases 0x50000 to the end and writes it (byte-identical on flash), next boot reaches login on 31:4ih_sizecorrupted in the kernel headerkernel header at 0x50000 fails its CRC, the same recovery, next boot reaches login on 31:4Bad Data CRC,can't get kernel image!, reset, with no recoveryBuilds for gk7205v500, hi3516cv300, hi3516ev300 and hi3516cv500 (no split support) with no new warnings. Not run on hardware.