[PATCH 0/6] ASoC: SOF: ipc3/ipc4-control: harden kcontrol payload handling

[PATCH 0/6] ASoC: SOF: ipc3/ipc4-control: harden kcontrol payload handling

[Peter Ujfalusi]

Hi,

This series hardens SOF kcontrol data paths for both IPC3 and IPC4 by
fixing size-handling bugs in put/get/update flows and tightening bounds
checks around firmware/user-provided payload lengths.

The changes include:

Fix TOCTOU-style size misuse in IPC3/IPC4 bytes put paths by validating and
using the incoming payload size.
Add notification/update payload size validation before parsing control data.
Use overflow-checked arithmetic when computing expected IPC3 control sizes.
Ensure update/copy bounds are validated against actual allocation limits.
Fix IPC3 bytes_ext bounds checks to account for struct header offset, closing
a heap overflow/over-read issue from unprivileged userspace TLV access.
Overall, the series makes control payload processing robust against malformed or
inconsistent sizes and prevents out-of-bounds accesses.

Regards,
Peter
---
Peter Ujfalusi (6):
  ASoC: SOF: ipc4-control: Fix TOCTOU in sof_ipc4_bytes_put
  ASoC: SOF: ipc4-control: Validate notification payload size
  ASoC: SOF: ipc3-control: Use overflow checks in control_update size
    calc
  ASoC: SOF: ipc3-control: Validate size in snd_sof_update_control
  ASoC: SOF: ipc3-control: Fix TOCTOU in bytes_put and bytes_get
  ASoC: SOF: ipc3-control: Fix heap overflow in bytes_ext put/get

 sound/soc/sof/ipc3-control.c | 79 +++++++++++++++++++++++++++---------
 sound/soc/sof/ipc4-control.c | 34 ++++++++++++++--
 2 files changed, 90 insertions(+), 23 deletions(-)

-- 
2.54.0

[PATCH 1/6] ASoC: SOF: ipc4-control: Fix TOCTOU in sof_ipc4_bytes_put

[Peter Ujfalusi]

In sof_ipc4_bytes_put(), the copy size is derived from the old
data->size in the buffer rather than the incoming new data's size
field from ucontrol. If the new data has a different size, the copy
uses the wrong length: it may truncate valid data or copy stale bytes.

Fix by validating and using the incoming data's sof_abi_hdr.size from
ucontrol before copying.

Fixes: a062c8899fed ("ASoC: SOF: ipc4-control: Add support for bytes control get and put")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/ipc4-control.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/ipc4-control.c b/sound/soc/sof/ipc4-control.c
index 4ce821f96a91..aa31eed05730 100644
--- a/sound/soc/sof/ipc4-control.c
+++ b/sound/soc/sof/ipc4-control.c
@@ -554,6 +554,8 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
 	struct snd_soc_component *scomp = scontrol->scomp;
 	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
 	struct sof_abi_hdr *data = cdata->data;
+	const struct sof_abi_hdr *new_hdr =
+		(const struct sof_abi_hdr *)ucontrol->value.bytes.data;
 	size_t size;
 	int ret;
 
@@ -564,15 +566,16 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
 		return -EINVAL;
 	}
 
-	/* scontrol->max_size has been verified to be >= sizeof(struct sof_abi_hdr) */
-	if (data->size > scontrol->max_size - sizeof(*data)) {
+	/* Validate the new data's size, not the old one */
+	if (new_hdr->size > scontrol->max_size - sizeof(*new_hdr)) {
 		dev_err_ratelimited(scomp->dev,
 				    "data size too big %u bytes max is %zu\n",
-				    data->size, scontrol->max_size - sizeof(*data));
+				    new_hdr->size,
+				    scontrol->max_size - sizeof(*new_hdr));
 		return -EINVAL;
 	}
 
-	size = data->size + sizeof(*data);
+	size = new_hdr->size + sizeof(*new_hdr);
 
 	/* copy from kcontrol */
 	memcpy(data, ucontrol->value.bytes.data, size);
-- 
2.54.0

[PATCH 2/6] ASoC: SOF: ipc4-control: Validate notification payload size

[Peter Ujfalusi]

Validate MODULE_NOTIFICATION payload length before reading
bytes/channel data in control update handling.

Fixes: 2a28b5240f2b ("ASoC: SOF: ipc4-control: Add support for generic bytes control")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/ipc4-control.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/sound/soc/sof/ipc4-control.c b/sound/soc/sof/ipc4-control.c
index aa31eed05730..8d86d32a16ca 100644
--- a/sound/soc/sof/ipc4-control.c
+++ b/sound/soc/sof/ipc4-control.c
@@ -875,6 +875,16 @@ static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
 		 */
 		if (type == SND_SOC_TPLG_TYPE_BYTES) {
 			struct sof_abi_hdr *data = cdata->data;
+			size_t source_size = struct_size(msg_data, data, msg_data->num_elems);
+
+			if (source_size > ndata->event_data_size) {
+				dev_warn(sdev->dev,
+					 "%s: invalid bytes notification size for %s (%zu, %u)\n",
+					 __func__, scontrol->name, source_size,
+					 ndata->event_data_size);
+				scontrol->comp_data_dirty = true;
+				goto notify;
+			}
 
 			if (msg_data->num_elems > scontrol->max_size - sizeof(*data)) {
 				dev_warn(sdev->dev,
@@ -887,6 +897,17 @@ static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
 				scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
 			}
 		} else {
+			size_t source_size = struct_size(msg_data, chanv, msg_data->num_elems);
+
+			if (source_size > ndata->event_data_size) {
+				dev_warn(sdev->dev,
+					 "%s: invalid channel notification size for %s (%zu, %u)\n",
+					 __func__, scontrol->name, source_size,
+					 ndata->event_data_size);
+				scontrol->comp_data_dirty = true;
+				goto notify;
+			}
+
 			for (i = 0; i < msg_data->num_elems; i++) {
 				u32 channel = msg_data->chanv[i].channel;
 
@@ -914,6 +935,8 @@ static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
 		scontrol->comp_data_dirty = true;
 	}
 
+notify:
+
 	/*
 	 * Look up the ALSA kcontrol of the scontrol to be able to send a
 	 * notification to user space
-- 
2.54.0

[PATCH 0/6] ASoC: SOF: ipc3/ipc4-control: harden kcontrol payload handling

[Peter Ujfalusi]

Hi,

This series hardens SOF kcontrol data paths for both IPC3 and IPC4 by
fixing size-handling bugs in put/get/update flows and tightening bounds
checks around firmware/user-provided payload lengths.

The changes include:

Fix TOCTOU-style size misuse in IPC3/IPC4 bytes put paths by validating and
using the incoming payload size.
Add notification/update payload size validation before parsing control data.
Use overflow-checked arithmetic when computing expected IPC3 control sizes.
Ensure update/copy bounds are validated against actual allocation limits.
Fix IPC3 bytes_ext bounds checks to account for struct header offset, closing
a heap overflow/over-read issue from unprivileged userspace TLV access.
Overall, the series makes control payload processing robust against malformed or
inconsistent sizes and prevents out-of-bounds accesses.

Regards,
Peter
---
Peter Ujfalusi (6):
  ASoC: SOF: ipc4-control: Fix TOCTOU in sof_ipc4_bytes_put
  ASoC: SOF: ipc4-control: Validate notification payload size
  ASoC: SOF: ipc3-control: Use overflow checks in control_update size
    calc
  ASoC: SOF: ipc3-control: Validate size in snd_sof_update_control
  ASoC: SOF: ipc3-control: Fix TOCTOU in bytes_put and bytes_get
  ASoC: SOF: ipc3-control: Fix heap overflow in bytes_ext put/get

 sound/soc/sof/ipc3-control.c | 79 +++++++++++++++++++++++++++---------
 sound/soc/sof/ipc4-control.c | 34 ++++++++++++++--
 2 files changed, 90 insertions(+), 23 deletions(-)

-- 
2.54.0

[PATCH 1/6] ASoC: SOF: ipc4-control: Fix TOCTOU in sof_ipc4_bytes_put

[Peter Ujfalusi]

In sof_ipc4_bytes_put(), the copy size is derived from the old
data->size in the buffer rather than the incoming new data's size
field from ucontrol. If the new data has a different size, the copy
uses the wrong length: it may truncate valid data or copy stale bytes.

Fix by validating and using the incoming data's sof_abi_hdr.size from
ucontrol before copying.

Fixes: a062c8899fed ("ASoC: SOF: ipc4-control: Add support for bytes control get and put")
Cc: stable@vger.kernel.org
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/ipc4-control.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/ipc4-control.c b/sound/soc/sof/ipc4-control.c
index 4ce821f96a91..aa31eed05730 100644
--- a/sound/soc/sof/ipc4-control.c
+++ b/sound/soc/sof/ipc4-control.c
@@ -554,6 +554,8 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
 	struct snd_soc_component *scomp = scontrol->scomp;
 	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
 	struct sof_abi_hdr *data = cdata->data;
+	const struct sof_abi_hdr *new_hdr =
+		(const struct sof_abi_hdr *)ucontrol->value.bytes.data;
 	size_t size;
 	int ret;
 
@@ -564,15 +566,16 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
 		return -EINVAL;
 	}
 
-	/* scontrol->max_size has been verified to be >= sizeof(struct sof_abi_hdr) */
-	if (data->size > scontrol->max_size - sizeof(*data)) {
+	/* Validate the new data's size, not the old one */
+	if (new_hdr->size > scontrol->max_size - sizeof(*new_hdr)) {
 		dev_err_ratelimited(scomp->dev,
 				    "data size too big %u bytes max is %zu\n",
-				    data->size, scontrol->max_size - sizeof(*data));
+				    new_hdr->size,
+				    scontrol->max_size - sizeof(*new_hdr));
 		return -EINVAL;
 	}
 
-	size = data->size + sizeof(*data);
+	size = new_hdr->size + sizeof(*new_hdr);
 
 	/* copy from kcontrol */
 	memcpy(data, ucontrol->value.bytes.data, size);
-- 
2.54.0