[PATCH] ASoC: SOF: ipc3: bound firmware-supplied ext header size

[PATCH] ASoC: SOF: ipc3: bound firmware-supplied ext header size

[Ștefan Ghețu]

ext_hdr->hdr.size comes straight from firmware and is used unchecked
as a read length: too large overflows the PAGE_SIZE heap allocation
in ext_data, too small underflows the size_t subtraction (hdr.size -
sizeof(*ext_hdr)), producing a read length near SIZE_MAX.

Bound hdr.size to [sizeof(*ext_hdr), PAGE_SIZE] before using it.

Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
---
 sound/soc/sof/ipc3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/sof/ipc3.c b/sound/soc/sof/ipc3.c
index 85bb22bbe18d..6188e43726e2 100644
--- a/sound/soc/sof/ipc3.c
+++ b/sound/soc/sof/ipc3.c
@@ -598,6 +598,14 @@ static int ipc3_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 offset)
 	ext_hdr = ext_data;
 
 	while (ext_hdr->hdr.cmd == SOF_IPC_FW_READY) {
+		/* bound hdr.size to avoid heap overflow/underflow */
+		if (ext_hdr->hdr.size < sizeof(*ext_hdr) ||
+		   ext_hdr->hdr.size > PAGE_SIZE) {
+			dev_err(sdev->dev, "invalid ext data size 0x%x\n",
+				ext_hdr->hdr.size);
+			ret = -EINVAL;
+			break;
+		}
 		/* read in ext structure */
 		snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM,
 				       offset + sizeof(*ext_hdr),
-- 
2.53.0

Re: [PATCH] ASoC: SOF: ipc3: bound firmware-supplied ext header size

[Péter Ujfalusi]

On 10/09/2026 00:23, Ștefan Ghețu wrote:
> ext_hdr->hdr.size comes straight from firmware and is used unchecked
> as a read length: too large overflows the PAGE_SIZE heap allocation
> in ext_data, too small underflows the size_t subtraction (hdr.size -
> sizeof(*ext_hdr)), producing a read length near SIZE_MAX.
> 
> Bound hdr.size to [sizeof(*ext_hdr), PAGE_SIZE] before using it.

We tend to trust the firmware on this.
You would need to compromise the system in so many layers before you
could even boot a firmware which is malicious to send deliberately wrong
information that this is the least of the issue...

You need to sign the firmware with a key which is accepted by the system
(the signing key is secret, even we developers have no access to it).
You need root access to deploy the firmware to system.

At this point, there is not really a need to brother with this, you can
do anything as root...

And if we are this paranoid, we cannot trust the ext_hdr->hdr.size size
either, the attacker can deploy the firmware which would pass this check.

I know, agents flags this (and other similar cases), but we are trying
to be realistic.

> 
> Signed-off-by: Ștefan Ghețu <stefanghetu9@gmail.com>
> ---
>  sound/soc/sof/ipc3.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/sound/soc/sof/ipc3.c b/sound/soc/sof/ipc3.c
> index 85bb22bbe18d..6188e43726e2 100644
> --- a/sound/soc/sof/ipc3.c
> +++ b/sound/soc/sof/ipc3.c
> @@ -598,6 +598,14 @@ static int ipc3_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 offset)
>  	ext_hdr = ext_data;
>  
>  	while (ext_hdr->hdr.cmd == SOF_IPC_FW_READY) {
> +		/* bound hdr.size to avoid heap overflow/underflow */
> +		if (ext_hdr->hdr.size < sizeof(*ext_hdr) ||
> +		   ext_hdr->hdr.size > PAGE_SIZE) {
> +			dev_err(sdev->dev, "invalid ext data size 0x%x\n",
> +				ext_hdr->hdr.size);
> +			ret = -EINVAL;
> +			break;
> +		}
>  		/* read in ext structure */
>  		snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM,
>  				       offset + sizeof(*ext_hdr),

-- 
Péter