Skip to content

Commit 5fa4400

Browse files
committed
tools: rimage: harden CSE header scan and re-sign cleanup
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>
1 parent e5c30de commit 5fa4400

1 file changed

Lines changed: 16 additions & 8 deletions

File tree

tools/rimage/src/manifest.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,23 @@ static bool cse_header_is_valid(const struct image *image, const void *buffer, s
3232
if (image->adsp->man_v2_5 || image->adsp->man_ace_v1_5) {
3333
const struct CsePartitionDirHeader_v2_5 *cse_hdr = buffer;
3434

35+
if (size < sizeof(*cse_hdr))
36+
return false;
37+
3538
return cse_hdr->header_marker == CSE_HEADER_MAKER &&
3639
cse_hdr->nb_entries == MAN_CSE_PARTS &&
37-
cse_hdr->header_length >= sizeof(*cse_hdr) &&
38-
size >= sizeof(*cse_hdr);
40+
cse_hdr->header_length >= sizeof(*cse_hdr);
3941
}
4042

41-
if (image->adsp->man_v1_5 || image->adsp->man_v1_8) {
43+
if (image->adsp->man_v1_5 || image->adsp->man_v1_5_sue || image->adsp->man_v1_8) {
4244
const struct CsePartitionDirHeader *cse_hdr = buffer;
4345

46+
if (size < sizeof(*cse_hdr))
47+
return false;
48+
4449
return cse_hdr->header_marker == CSE_HEADER_MAKER &&
4550
cse_hdr->nb_entries == MAN_CSE_PARTS &&
46-
cse_hdr->header_length >= sizeof(*cse_hdr) &&
47-
size >= sizeof(*cse_hdr);
51+
cse_hdr->header_length >= sizeof(*cse_hdr);
4852
}
4953

5054
return false;
@@ -1669,7 +1673,7 @@ int verify_image(struct image *image)
16691673
{
16701674
FILE *in_file;
16711675
int ret = -EINVAL;
1672-
void *buffer;
1676+
void *buffer = NULL;
16731677
size_t size, read, i;
16741678

16751679
/* is verify supported for target ? */
@@ -1842,11 +1846,15 @@ int resign_image(struct image *image)
18421846
if (ret < 0)
18431847
goto out;
18441848

1845-
if (fclose(image->out_fd)) {
1849+
/* fclose() closes the stream even when it reports an error, so clear the
1850+
* handle first to stop the caller's cleanup from closing it a second time.
1851+
*/
1852+
ret = fclose(image->out_fd);
1853+
image->out_fd = NULL;
1854+
if (ret) {
18461855
ret = file_error("unable to close file after signing", image->out_file);
18471856
goto out;
18481857
}
1849-
image->out_fd = NULL;
18501858

18511859
/* validate the re-signed output with the same private key */
18521860
image->verify_file = image->out_file;

0 commit comments

Comments
 (0)