Skip to content

Fixes for re-signing images and dumping metadata.#10827

Open
lgirdwood wants to merge 9 commits into
thesofproject:mainfrom
lgirdwood:topic/resign
Open

Fixes for re-signing images and dumping metadata.#10827
lgirdwood wants to merge 9 commits into
thesofproject:mainfrom
lgirdwood:topic/resign

Conversation

@lgirdwood

Copy link
Copy Markdown
Member

rimage resign was not working for certain devices alongside the manifest dump tooling needed updates for newer platforms too.

Copilot AI review requested due to automatic review settings May 29, 2026 17:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates rimage signing/verification flows and extends the firmware manifest dumper for newer platform metadata formats.

Changes:

  • Improves rimage verify/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.

Comment on lines +854 to +863
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()))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment thread tools/sof_ri_info/sof_ri_info.py Outdated
Comment on lines +1290 to +1292
for i in range(0, num_module_entries):
mod_cfg = parse_adsp_manifest_mod_config(i, reader)
adsp_mft.add_comp(mod_cfg)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +32 to +48
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);
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tools/rimage/src/manifest.c Outdated
Comment on lines +1813 to +1817
if (fclose(image->out_fd)) {
ret = file_error("unable to close file after signing", image->out_file);
goto out;
}
image->out_fd = NULL;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

lrgirdwo added 9 commits June 19, 2026 17:10
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants