ASoC: SOF: ipc4-control: Use local copy of IPC message for sending#5737
ASoC: SOF: ipc4-control: Use local copy of IPC message for sending#5737ujfalusi wants to merge 1 commit intothesofproject:topic/sof-devfrom
Conversation
ujfalusi
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
plbossart and
ranj063
as code owners
There was a problem hiding this comment.
Pull request overview
Moves IPC4 kcontrol message payload/param configuration into sof_ipc4_set_get_kcontrol_data() so it happens under swidget->setup_mutex, preventing races between control updates and widget/PCM startup.
Changes:
- Extended
sof_ipc4_set_get_kcontrol_data()to accept payload pointer/size and a param_id override. - Updated volume, generic, and bytes control paths to pass payload/config directly into the helper instead of mutating
cdata->msgin the callers. - Removed per-caller clearing of
msg->data_ptr/msg->data_sizeafter IPC operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| msg->data_ptr = data; | ||
| msg->data_size = data_size; | ||
| ret = iops->set_get_data(sdev, msg, msg->data_size, set); |
There was a problem hiding this comment.
msg->data_ptr and msg->data_size are now set here but never reset. Several callers pass stack or temporary heap buffers (e.g. ¶ms, kzalloc()ed payloads) that are freed/invalid after this call, leaving a dangling pointer in cdata->msg that can be observed/reused later. Clear msg->data_ptr/msg->data_size (and, if applicable, restore any overridden fields) before releasing setup_mutex to avoid stale state and potential UAF if future code paths reuse the message template without reinitializing these fields.
| if (param_id) | ||
| msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(param_id); |
There was a problem hiding this comment.
param_id is treated as “optional” via if (param_id), which breaks bytes controls when the ABI header’s type (used as param_id) is 0. sof_ipc4_bytes_put() copies sof_abi_hdr from userspace, so data->type can legitimately become 0; in that case this function will leave msg->extension set to the previous non-zero param_id and send the request with the wrong parameter ID. Use an explicit flag to indicate whether to override the param_id (or always apply the provided param_id, including 0, and have non-bytes callers pass a separate sentinel/flag when they want to preserve the template extension).
ujfalusi
force-pushed
the
peter/sof/pr/ipc4-control-init-race
branch
2 times, most recently
from
ece5125 to
d50d9d5
Compare
If a kcontrol update comes to a control right at the same time when the PCM containing the control is started up then there is a small window when a race can happen: while the widget is set up the swidget->setup_mutex is taken in topology level and if the control update comes at this point, it will be stopped within sof_ipc4_set_get_kcontrol_data() with the mutex and it will be blocked until the swidget setup is done, which will clear the control's IPC message payload. To avoid such race, use local copy of the template IPC message instead of the template directly. This will ensure data integrity in case of concurrent updates during initialization. Link: thesofproject#5734 Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
ujfalusi
changed the title
|
Changes since v1:
|
If a kcontrol update comes to a control right at the same time when the PCM containing the control is started up then there is a small window when a race can happen:
while the widget is set up the swidget->setup_mutex is taken in topology level and if the control update comes at this point, it will be stopped within sof_ipc4_set_get_kcontrol_data() with the mutex and it will be blocked until the swidget setup is done, which will clear the control's IPC message payload.
To avoid such race, use local copy of the template IPC message instead of the template directly. This will ensure data integrity in case of concurrent updates during initialization.
Link: #5734