Skip to content

Commit 3019b86

Browse files
rfvirgilbroonie
authored andcommitted
firmware: cs_dsp: Fix overflow checking of wmfw header
Fix the checking that firmware file buffer is large enough for the wmfw header, to prevent overrunning the buffer. The original code tested that the firmware data buffer contained enough bytes for the sums of the size of the structs wmfw_header + wmfw_adsp1_sizes + wmfw_footer But wmfw_adsp1_sizes is only used on ADSP1 firmware. For ADSP2 and Halo Core the equivalent struct is wmfw_adsp2_sizes, which is 4 bytes longer. So the length check didn't guarantee that there are enough bytes in the firmware buffer for a header with wmfw_adsp2_sizes. This patch splits the length check into three separate parts. Each of the wmfw_header, wmfw_adsp?_sizes and wmfw_footer are checked separately before they are used. Signed-off-by: Richard Fitzgerald <[email protected]> Fixes: f6bc909 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs") Link: https://patch.msgid.link/[email protected] Signed-off-by: Mark Brown <[email protected]>
1 parent 68f97fe commit 3019b86

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

drivers/firmware/cirrus/cs_dsp.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,6 +1321,10 @@ static unsigned int cs_dsp_adsp1_parse_sizes(struct cs_dsp *dsp,
13211321
const struct wmfw_adsp1_sizes *adsp1_sizes;
13221322

13231323
adsp1_sizes = (void *)&firmware->data[pos];
1324+
if (sizeof(*adsp1_sizes) > firmware->size - pos) {
1325+
cs_dsp_err(dsp, "%s: file truncated\n", file);
1326+
return 0;
1327+
}
13241328

13251329
cs_dsp_dbg(dsp, "%s: %d DM, %d PM, %d ZM\n", file,
13261330
le32_to_cpu(adsp1_sizes->dm), le32_to_cpu(adsp1_sizes->pm),
@@ -1337,6 +1341,10 @@ static unsigned int cs_dsp_adsp2_parse_sizes(struct cs_dsp *dsp,
13371341
const struct wmfw_adsp2_sizes *adsp2_sizes;
13381342

13391343
adsp2_sizes = (void *)&firmware->data[pos];
1344+
if (sizeof(*adsp2_sizes) > firmware->size - pos) {
1345+
cs_dsp_err(dsp, "%s: file truncated\n", file);
1346+
return 0;
1347+
}
13401348

13411349
cs_dsp_dbg(dsp, "%s: %d XM, %d YM %d PM, %d ZM\n", file,
13421350
le32_to_cpu(adsp2_sizes->xm), le32_to_cpu(adsp2_sizes->ym),
@@ -1376,7 +1384,6 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
13761384
struct regmap *regmap = dsp->regmap;
13771385
unsigned int pos = 0;
13781386
const struct wmfw_header *header;
1379-
const struct wmfw_adsp1_sizes *adsp1_sizes;
13801387
const struct wmfw_footer *footer;
13811388
const struct wmfw_region *region;
13821389
const struct cs_dsp_region *mem;
@@ -1392,10 +1399,8 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
13921399

13931400
ret = -EINVAL;
13941401

1395-
pos = sizeof(*header) + sizeof(*adsp1_sizes) + sizeof(*footer);
1396-
if (pos >= firmware->size) {
1397-
cs_dsp_err(dsp, "%s: file too short, %zu bytes\n",
1398-
file, firmware->size);
1402+
if (sizeof(*header) >= firmware->size) {
1403+
ret = -EOVERFLOW;
13991404
goto out_fw;
14001405
}
14011406

@@ -1423,13 +1428,16 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
14231428

14241429
pos = sizeof(*header);
14251430
pos = dsp->ops->parse_sizes(dsp, file, pos, firmware);
1431+
if ((pos == 0) || (sizeof(*footer) > firmware->size - pos)) {
1432+
ret = -EOVERFLOW;
1433+
goto out_fw;
1434+
}
14261435

14271436
footer = (void *)&firmware->data[pos];
14281437
pos += sizeof(*footer);
14291438

14301439
if (le32_to_cpu(header->len) != pos) {
1431-
cs_dsp_err(dsp, "%s: unexpected header length %d\n",
1432-
file, le32_to_cpu(header->len));
1440+
ret = -EOVERFLOW;
14331441
goto out_fw;
14341442
}
14351443

@@ -1555,6 +1563,9 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
15551563
cs_dsp_buf_free(&buf_list);
15561564
kfree(text);
15571565

1566+
if (ret == -EOVERFLOW)
1567+
cs_dsp_err(dsp, "%s: file content overflows file data\n", file);
1568+
15581569
return ret;
15591570
}
15601571

0 commit comments

Comments
 (0)