Fixes for re-signing images and dumping metadata.#10827
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates rimage signing/verification flows and extends the firmware manifest dumper for newer platform metadata formats.
Changes:
- Improves
rimageverify/re-sign mode handling, including output validation and CLI mode exclusivity. - Adds parsing/dumping support for newer CSE/CSS/ADSP manifest fields and extensions.
- Adds memory map entries for newer Intel platforms.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
tools/rimage/src/rimage.c |
Updates CLI help and rejects simultaneous verify/re-sign modes. |
tools/rimage/src/manifest.c |
Adds stricter CSE header detection and validates re-signed output. |
tools/sof_ri_info/sof_ri_info.py |
Extends manifest parsing, dumping, module config handling, and platform memory maps. |
| legacy_or_unused = reader.read_b() | ||
| if header_length > 12: | ||
| hdr.add_a(Ahex('not_used', legacy_or_unused)) | ||
| else: | ||
| hdr.add_a(Ahex('checksum', legacy_or_unused)) | ||
| hdr.add_a(Astring('partition_name', reader.read_string(4))) | ||
|
|
||
| # CSE v2.5 extends header with a CRC32 checksum dword | ||
| if header_length > 12: | ||
| hdr.add_a(Ahex('checksum32', reader.read_dw())) |
There was a problem hiding this comment.
Fixed in 1857434. The discriminator now keys off the v2.5 header length (header_length >= 20) instead of > 12, so the 16-byte legacy v1.x header stays on the single-byte checksum path and is no longer mis-parsed (no spurious not_used/checksum32).
| for i in range(0, num_module_entries): | ||
| mod_cfg = parse_adsp_manifest_mod_config(i, reader) | ||
| adsp_mft.add_comp(mod_cfg) |
There was a problem hiding this comment.
Fixed in 1857434. We now sum each module entry's cfg_count and dump that many configs, rather than assuming one config per module entry, so additional configurations are no longer dropped.
| if (image->adsp->man_v2_5 || image->adsp->man_ace_v1_5) { | ||
| const struct CsePartitionDirHeader_v2_5 *cse_hdr = buffer; | ||
|
|
||
| return cse_hdr->header_marker == CSE_HEADER_MAKER && | ||
| cse_hdr->nb_entries == MAN_CSE_PARTS && | ||
| cse_hdr->header_length >= sizeof(*cse_hdr) && | ||
| size >= sizeof(*cse_hdr); | ||
| } | ||
|
|
||
| if (image->adsp->man_v1_5 || image->adsp->man_v1_8) { | ||
| const struct CsePartitionDirHeader *cse_hdr = buffer; | ||
|
|
||
| return cse_hdr->header_marker == CSE_HEADER_MAKER && | ||
| cse_hdr->nb_entries == MAN_CSE_PARTS && | ||
| cse_hdr->header_length >= sizeof(*cse_hdr) && | ||
| size >= sizeof(*cse_hdr); | ||
| } |
There was a problem hiding this comment.
Fixed in 5fa4400. The size < sizeof(*cse_hdr) check now happens before the header is dereferenced, so a short/truncated tail can't be read past, and man_v1_5_sue is included in the v1.5 verifier set.
| fprintf(stderr, "error: could not find valid CSE header $CPD in %s\n", | ||
| image->verify_file); | ||
| out: | ||
| free(buffer); |
There was a problem hiding this comment.
Fixed in 5fa4400. buffer is initialised to NULL, so an early get_file_size() failure that jumps to the cleanup no longer calls free() on an uninitialised pointer.
| if (fclose(image->out_fd)) { | ||
| ret = file_error("unable to close file after signing", image->out_file); | ||
| goto out; | ||
| } | ||
| image->out_fd = NULL; |
There was a problem hiding this comment.
Fixed in 5fa4400. image->out_fd is cleared immediately after fclose() and before the error is handled, so the caller cleanup will not close the same stream a second time.
Avoid false-positive CPD header detection when re-signing by validating CSE header candidates before accepting them. Make verification failures propagate as non-zero return values and ensure resources are cleaned up on all paths. After re-signing, verify the generated image immediately and remove the output file if verification fails to prevent leaving broken artifacts. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Update command-line handling and help text to make resign usage clearer. Reject mutually exclusive mode combinations (resign with verify) early, so users get deterministic behavior and clearer error reporting. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Extend CSS and CSE parsing/dump paths to include fields that were previously skipped or only partially represented. Cover version-specific CSE fields and expose additional CSS header members so output better matches the current rimage headers. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add full parsing support for plat_auth extensions and their module entries, including partition info, signed package variants, and info 0x16. Update extension dumps to print these parsed fields and nested structures. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Parse additional ADSP metadata and manifest structures from the user manifest definitions, including module config records. Improve field naming/alignment with rimage structures while keeping compatibility aliases where needed. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Teach memory-map lookup about newer platform aliases used in firmware paths so known layouts are selected instead of the unknown fallback. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Introduce a PTL-specific memory map label and map PTL/NVL platform identifiers to it. This keeps displayed platform naming accurate while preserving the current address/size values. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Address review feedback on the verify/re-sign paths: - cse_header_is_valid() dereferenced the candidate header before confirming the remaining buffer was large enough, so scanning near the end of a short or truncated file could read past the allocation. Check the size first, then inspect the fields. Also include man_v1_5_sue in the v1.5 verifier set so SUE verify/re-sign scans accept valid headers. - verify_image() left buffer uninitialised, so an early get_file_size() failure reached the free(buffer) cleanup with a garbage pointer. Initialise it to NULL. - resign_image() left image->out_fd non-NULL when fclose() reported an error, even though the stream is already closed, leading the caller cleanup to close the same FILE * twice. Clear the handle before testing the result. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Address review feedback on the manifest dumper: - The legacy v1.x CsePartitionDirHeader is 16 bytes while the v2.5 header is 20 bytes, so a "header_length > 12" test sent legacy images down the v2.5 path, mislabelling their checksum byte as not_used and consuming the first entry dword as checksum32. Discriminate on the v2.5 header length (>= 20) so 16-byte headers stay on the legacy path. - Module configs are not one-per-module-entry: each module advertises its own cfg_count and the configs are packed contiguously, so the total written can exceed num_module_entries. Sum every module's cfg_count and dump that many configs instead of dropping the extras. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
rimage resign was not working for certain devices alongside the manifest dump tooling needed updates for newer platforms too.