From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: <linux-kernel@vger.kernel.org>, <patches@opensource.cirrus.com>,
<alsa-devel@alsa-project.org>, <linux-sound@vger.kernel.org>
Subject: [PATCH 1/4] firmware: cs_dsp: Fix overflow checking of wmfw header
Date: Thu, 27 Jun 2024 15:14:29 +0100 [thread overview]
Message-ID: <20240627141432.93056-2-rf@opensource.cirrus.com> (raw)
In-Reply-To: <20240627141432.93056-1-rf@opensource.cirrus.com>
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 <rf@opensource.cirrus.com>
Fixes: f6bc909e7673 ("firmware: cs_dsp: add driver to support firmware loading on Cirrus Logic DSPs")
---
drivers/firmware/cirrus/cs_dsp.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
index 0d139e4de37c..6eca62d31e20 100644
--- a/drivers/firmware/cirrus/cs_dsp.c
+++ b/drivers/firmware/cirrus/cs_dsp.c
@@ -1321,6 +1321,10 @@ static unsigned int cs_dsp_adsp1_parse_sizes(struct cs_dsp *dsp,
const struct wmfw_adsp1_sizes *adsp1_sizes;
adsp1_sizes = (void *)&firmware->data[pos];
+ if (sizeof(*adsp1_sizes) > firmware->size - pos) {
+ cs_dsp_err(dsp, "%s: file truncated\n", file);
+ return 0;
+ }
cs_dsp_dbg(dsp, "%s: %d DM, %d PM, %d ZM\n", file,
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,
const struct wmfw_adsp2_sizes *adsp2_sizes;
adsp2_sizes = (void *)&firmware->data[pos];
+ if (sizeof(*adsp2_sizes) > firmware->size - pos) {
+ cs_dsp_err(dsp, "%s: file truncated\n", file);
+ return 0;
+ }
cs_dsp_dbg(dsp, "%s: %d XM, %d YM %d PM, %d ZM\n", file,
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,
struct regmap *regmap = dsp->regmap;
unsigned int pos = 0;
const struct wmfw_header *header;
- const struct wmfw_adsp1_sizes *adsp1_sizes;
const struct wmfw_footer *footer;
const struct wmfw_region *region;
const struct cs_dsp_region *mem;
@@ -1392,10 +1399,8 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
ret = -EINVAL;
- pos = sizeof(*header) + sizeof(*adsp1_sizes) + sizeof(*footer);
- if (pos >= firmware->size) {
- cs_dsp_err(dsp, "%s: file too short, %zu bytes\n",
- file, firmware->size);
+ if (sizeof(*header) >= firmware->size) {
+ ret = -EOVERFLOW;
goto out_fw;
}
@@ -1423,13 +1428,16 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
pos = sizeof(*header);
pos = dsp->ops->parse_sizes(dsp, file, pos, firmware);
+ if ((pos == 0) || (sizeof(*footer) > firmware->size - pos)) {
+ ret = -EOVERFLOW;
+ goto out_fw;
+ }
footer = (void *)&firmware->data[pos];
pos += sizeof(*footer);
if (le32_to_cpu(header->len) != pos) {
- cs_dsp_err(dsp, "%s: unexpected header length %d\n",
- file, le32_to_cpu(header->len));
+ ret = -EOVERFLOW;
goto out_fw;
}
@@ -1555,6 +1563,9 @@ static int cs_dsp_load(struct cs_dsp *dsp, const struct firmware *firmware,
cs_dsp_buf_free(&buf_list);
kfree(text);
+ if (ret == -EOVERFLOW)
+ cs_dsp_err(dsp, "%s: file content overflows file data\n", file);
+
return ret;
}
--
2.39.2
next prev parent reply other threads:[~2024-06-27 14:14 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-27 14:14 [PATCH 0/4] firmware: cs_dsp: Add check to prevent overrunning the firmware file Richard Fitzgerald
2024-06-27 14:14 ` Richard Fitzgerald [this message]
2024-06-27 14:14 ` [PATCH 2/4] firmware: cs_dsp: Return error if block header overflows file Richard Fitzgerald
2024-06-27 14:14 ` [PATCH 3/4] firmware: cs_dsp: Validate payload length before processing block Richard Fitzgerald
2024-06-27 14:14 ` [PATCH 4/4] firmware: cs_dsp: Prevent buffer overrun when processing V2 alg headers Richard Fitzgerald
2024-07-01 16:08 ` [PATCH 0/4] firmware: cs_dsp: Add check to prevent overrunning the firmware file Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240627141432.93056-2-rf@opensource.cirrus.com \
--to=rf@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox