Skip to content

SoC: SOF: Fix control data bounds checking and TOCTOU vulnerabilities #5772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ujfalusi wants to merge 5 commits into
base: topic/sof-dev
Choose a base branch
from
Open

SoC: SOF: Fix control data bounds checking and TOCTOU vulnerabilities #5772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
159eeb5
ASoC: SOF: ipc4-control: Fix TOCTOU in sof_ipc4_bytes_put
ujfalusi May 12, 2026
b7b66fe
ASoC: SOF: ipc3-control: Use overflow checks in control_update size calc
ujfalusi May 12, 2026
362c05f
ASoC: SOF: ipc3-control: Validate size in snd_sof_update_control
ujfalusi May 12, 2026
1d7bc1e
ASoC: SOF: ipc3-control: Fix TOCTOU in bytes_put and bytes_get
ujfalusi May 12, 2026
fdbdc1b
ASoC: SOF: ipc3-control: Fix heap overflow in bytes_ext put/get
ujfalusi May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 60 additions & 19 deletions sound/soc/sof/ipc3-control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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)) {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Comment on lines 446 to 450
Copy link
Copy Markdown
Collaborator Author

@ujfalusi ujfalusi May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will change the check to this.

Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Comment on lines +564 to 568
Copy link
Copy Markdown
Collaborator Author

@ujfalusi ujfalusi May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect. In the binary path, cdata->num_elems is the total binary blob size copied from cdata->data (the sof_abi_hdr start). The existing num_elems == local_cdata->data->size check is how the original code has always worked — num_elems includes the ABI header bytes in this context.

} else if (cdata->num_elems != scontrol->num_channels) {
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 7 additions & 4 deletions sound/soc/sof/ipc4-control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Loading