ASoC: SOF: ipc3/ipc4-control: harden kcontrol payload handling

Peter Ujfalusi <peter.ujfalusi@linux.intel.com> says:

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.

Link: https://patch.msgid.link/20260609083458.31193-1-peter.ujfalusi@linux.intel.com
This commit is contained in:
Mark Brown 2026-06-09 18:41:15 +01:00
commit 9f0b311829
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0
2 changed files with 90 additions and 23 deletions

View File

@ -315,10 +315,13 @@ static int sof_ipc3_bytes_get(struct snd_sof_control *scontrol,
}
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
if (data->size > scontrol->max_size - sizeof(*data)) {
if (data->size > scontrol->max_size - sizeof(*cdata) -
sizeof(*data)) {
dev_err_ratelimited(scomp->dev,
"%u bytes of control data is invalid, max is %zu\n",
data->size, scontrol->max_size - sizeof(*data));
data->size,
scontrol->max_size - sizeof(*cdata) -
sizeof(*data));
return -EINVAL;
}
@ -336,6 +339,8 @@ static int sof_ipc3_bytes_put(struct snd_sof_control *scontrol,
struct sof_ipc_ctrl_data *cdata = scontrol->ipc_control_data;
struct snd_soc_component *scomp = scontrol->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;
if (scontrol->max_size > sizeof(ucontrol->value.bytes.data)) {
@ -344,14 +349,18 @@ static int sof_ipc3_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)) {
dev_err_ratelimited(scomp->dev, "data size too big %u bytes max is %zu\n",
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(*cdata) -
sizeof(*new_hdr)) {
dev_err_ratelimited(scomp->dev,
"data size too big %u bytes max is %zu\n",
new_hdr->size,
scontrol->max_size - sizeof(*cdata) -
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);
@ -389,9 +398,17 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol,
}
/* be->max is coming from topology */
if (header.length > scontrol->max_size) {
dev_err_ratelimited(scomp->dev, "Bytes data size %d exceeds max %zu\n",
header.length, scontrol->max_size);
if (header.length > scontrol->max_size - sizeof(*cdata)) {
dev_err_ratelimited(scomp->dev, "Bytes data size %u exceeds max %zu\n",
header.length, scontrol->max_size - sizeof(*cdata));
return -EINVAL;
}
/* Ensure the data is large enough to contain the ABI header */
if (header.length < sizeof(struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev,
"Bytes data size %u less than ABI header %zu\n",
header.length, sizeof(struct sof_abi_hdr));
return -EINVAL;
}
@ -427,7 +444,7 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol,
}
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
if (cdata->data->size > scontrol->max_size - sizeof(struct sof_abi_hdr)) {
if (cdata->data->size > scontrol->max_size - sizeof(*cdata) - sizeof(struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev, "Mismatch in ABI data size (truncated?)\n");
goto err_restore;
}
@ -443,7 +460,7 @@ static int sof_ipc3_bytes_ext_put(struct snd_sof_control *scontrol,
err_restore:
/* If we have an issue, we restore the old, valid bytes control data */
if (scontrol->old_ipc_control_data) {
memcpy(cdata->data, scontrol->old_ipc_control_data, scontrol->max_size);
memcpy(cdata, scontrol->old_ipc_control_data, scontrol->max_size);
kfree(scontrol->old_ipc_control_data);
scontrol->old_ipc_control_data = NULL;
}
@ -482,10 +499,13 @@ static int _sof_ipc3_bytes_ext_get(struct snd_sof_control *scontrol,
}
/* check data size doesn't exceed max coming from topology */
if (cdata->data->size > scontrol->max_size - sizeof(struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev, "User data size %d exceeds max size %zu\n",
if (cdata->data->size > scontrol->max_size - sizeof(*cdata) -
sizeof(struct sof_abi_hdr)) {
dev_err_ratelimited(scomp->dev,
"User data size %u exceeds max size %zu\n",
cdata->data->size,
scontrol->max_size - sizeof(struct sof_abi_hdr));
scontrol->max_size - sizeof(*cdata) -
sizeof(struct sof_abi_hdr));
return -EINVAL;
}
@ -535,6 +555,15 @@ static void snd_sof_update_control(struct snd_sof_control *scontrol,
return;
}
/* Verify the size fits within the allocation */
if (cdata->num_elems > scontrol->max_size - sizeof(*local_cdata) -
sizeof(*local_cdata->data)) {
dev_err(scomp->dev,
"cdata binary size %u exceeds buffer\n",
cdata->num_elems);
return;
}
/* copy the new binary data */
memcpy(local_cdata->data, cdata->data, cdata->num_elems);
} else if (cdata->num_elems != scontrol->num_channels) {
@ -626,16 +655,28 @@ static void sof_ipc3_control_update(struct snd_sof_dev *sdev, void *ipc_control_
return;
}
expected_size = sizeof(struct sof_ipc_ctrl_data);
switch (cdata->type) {
case SOF_CTRL_TYPE_VALUE_CHAN_GET:
case SOF_CTRL_TYPE_VALUE_CHAN_SET:
expected_size += cdata->num_elems *
sizeof(struct sof_ipc_ctrl_value_chan);
if (check_mul_overflow((size_t)cdata->num_elems,
sizeof(struct sof_ipc_ctrl_value_chan),
&expected_size))
return;
if (check_add_overflow(expected_size,
sizeof(struct sof_ipc_ctrl_data),
&expected_size))
return;
break;
case SOF_CTRL_TYPE_DATA_GET:
case SOF_CTRL_TYPE_DATA_SET:
expected_size += cdata->num_elems + sizeof(struct sof_abi_hdr);
if (check_add_overflow((size_t)cdata->num_elems,
sizeof(struct sof_abi_hdr),
&expected_size))
return;
if (check_add_overflow(expected_size,
sizeof(struct sof_ipc_ctrl_data),
&expected_size))
return;
break;
default:
return;

View File

@ -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);
@ -872,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,
@ -884,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;
@ -911,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