Skip to content

Commit b6f95f1

Browse files
ujfalusibardliao
authored andcommitted
ASoC: SOF: ipc3-control: Fix TOCTOU in bytes_put and bytes_get
In sof_ipc3_bytes_put(), the size used for the memcpy is derived from the old data->size already in the buffer, not the incoming new data's size field. If the new data has a different size, the copy length is wrong: it may truncate valid data or copy stale bytes. Similarly, sof_ipc3_bytes_get() checks data->size against max_size without accounting for the sizeof(struct sof_ipc_ctrl_data) offset of the flex array within the allocation. Fix bytes_put to validate and use the incoming data's sof_abi_hdr.size from ucontrol before copying. Fix bytes_get to subtract sizeof(*cdata) from the bounds check to match the actual available space. Fixes: 544ac88 ("ASoC: SOF: Add bytes_get/put control IPC ops for IPC3") Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
1 parent 4ef2a35 commit b6f95f1

1 file changed

Lines changed: 16 additions & 7 deletions

File tree

sound/soc/sof/ipc3-control.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,13 @@ static int sof_ipc3_bytes_get(struct snd_sof_control *scontrol,
315315
}
316316

317317
/* be->max has been verified to be >= sizeof(struct sof_abi_hdr) */
318-
if (data->size > scontrol->max_size - sizeof(*data)) {
318+
if (data->size > scontrol->max_size - sizeof(*cdata) -
319+
sizeof(*data)) {
319320
dev_err_ratelimited(scomp->dev,
320321
"%u bytes of control data is invalid, max is %zu\n",
321-
data->size, scontrol->max_size - sizeof(*data));
322+
data->size,
323+
scontrol->max_size - sizeof(*cdata) -
324+
sizeof(*data));
322325
return -EINVAL;
323326
}
324327

@@ -336,6 +339,8 @@ static int sof_ipc3_bytes_put(struct snd_sof_control *scontrol,
336339
struct sof_ipc_ctrl_data *cdata = scontrol->ipc_control_data;
337340
struct snd_soc_component *scomp = scontrol->scomp;
338341
struct sof_abi_hdr *data = cdata->data;
342+
const struct sof_abi_hdr *new_hdr =
343+
(const struct sof_abi_hdr *)ucontrol->value.bytes.data;
339344
size_t size;
340345

341346
if (scontrol->max_size > sizeof(ucontrol->value.bytes.data)) {
@@ -344,14 +349,18 @@ static int sof_ipc3_bytes_put(struct snd_sof_control *scontrol,
344349
return -EINVAL;
345350
}
346351

347-
/* scontrol->max_size has been verified to be >= sizeof(struct sof_abi_hdr) */
348-
if (data->size > scontrol->max_size - sizeof(*data)) {
349-
dev_err_ratelimited(scomp->dev, "data size too big %u bytes max is %zu\n",
350-
data->size, scontrol->max_size - sizeof(*data));
352+
/* Validate the new data's size, not the old one */
353+
if (new_hdr->size > scontrol->max_size - sizeof(*cdata) -
354+
sizeof(*new_hdr)) {
355+
dev_err_ratelimited(scomp->dev,
356+
"data size too big %u bytes max is %zu\n",
357+
new_hdr->size,
358+
scontrol->max_size - sizeof(*cdata) -
359+
sizeof(*new_hdr));
351360
return -EINVAL;
352361
}
353362

354-
size = data->size + sizeof(*data);
363+
size = new_hdr->size + sizeof(*new_hdr);
355364

356365
/* copy from kcontrol */
357366
memcpy(data, ucontrol->value.bytes.data, size);

0 commit comments

Comments
 (0)