Skip to content

Commit 5d9be4e

Browse files
ujfalusibardliao
ujfalusi
authored and
bardliao
committed
ASoC: SOF: ipc4-control: Use local copy of IPC message for sending
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 Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
1 parent 774fe04 commit 5d9be4e

1 file changed

Lines changed: 41 additions & 49 deletions

File tree

‎sound/soc/sof/ipc4-control.c‎

Lines changed: 41 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@
1313
#include "ipc4-topology.h"
1414

1515
static int sof_ipc4_set_get_kcontrol_data(struct snd_sof_control *scontrol,
16+
struct sof_ipc4_msg *msg,
1617
bool set, bool lock)
1718
{
18-
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
1919
struct snd_soc_component *scomp = scontrol->scomp;
2020
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
2121
const struct sof_ipc_ops *iops = sdev->ipc->ops;
22-
struct sof_ipc4_msg *msg = &cdata->msg;
2322
struct snd_sof_widget *swidget;
2423
bool widget_found = false;
2524
int ret = 0;
@@ -88,9 +87,9 @@ sof_ipc4_set_volume_data(struct snd_sof_dev *sdev, struct snd_sof_widget *swidge
8887
{
8988
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
9089
struct sof_ipc4_gain *gain = swidget->private;
91-
struct sof_ipc4_msg *msg = &cdata->msg;
9290
struct sof_ipc4_gain_params params;
9391
bool all_channels_equal = true;
92+
struct sof_ipc4_msg msg;
9493
u32 value;
9594
int ret, i;
9695

@@ -107,6 +106,7 @@ sof_ipc4_set_volume_data(struct snd_sof_dev *sdev, struct snd_sof_widget *swidge
107106
* notify DSP with a single IPC message if all channel values are equal. Otherwise send
108107
* a separate IPC for each channel.
109108
*/
109+
memcpy(&msg, &cdata->msg, sizeof(msg));
110110
for (i = 0; i < scontrol->num_channels; i++) {
111111
if (all_channels_equal) {
112112
params.channels = SOF_IPC4_GAIN_ALL_CHANNELS_MASK;
@@ -121,12 +121,10 @@ sof_ipc4_set_volume_data(struct snd_sof_dev *sdev, struct snd_sof_widget *swidge
121121
params.curve_duration_h = gain->data.params.curve_duration_h;
122122
params.curve_type = gain->data.params.curve_type;
123123

124-
msg->data_ptr = &params;
125-
msg->data_size = sizeof(params);
124+
msg.data_ptr = &params;
125+
msg.data_size = sizeof(params);
126126

127-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, true, lock);
128-
msg->data_ptr = NULL;
129-
msg->data_size = 0;
127+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, true, lock);
130128
if (ret < 0) {
131129
dev_err(sdev->dev, "Failed to set volume update for %s\n",
132130
scontrol->name);
@@ -208,7 +206,7 @@ sof_ipc4_set_generic_control_data(struct snd_sof_dev *sdev,
208206
{
209207
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
210208
struct sof_ipc4_control_msg_payload *data;
211-
struct sof_ipc4_msg *msg = &cdata->msg;
209+
struct sof_ipc4_msg msg;
212210
size_t data_size;
213211
unsigned int i;
214212
int ret;
@@ -225,12 +223,11 @@ sof_ipc4_set_generic_control_data(struct snd_sof_dev *sdev,
225223
data->chanv[i].value = cdata->chanv[i].value;
226224
}
227225

228-
msg->data_ptr = data;
229-
msg->data_size = data_size;
226+
memcpy(&msg, &cdata->msg, sizeof(msg));
227+
msg.data_ptr = data;
228+
msg.data_size = data_size;
230229

231-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, true, lock);
232-
msg->data_ptr = NULL;
233-
msg->data_size = 0;
230+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, true, lock);
234231
if (ret < 0)
235232
dev_err(sdev->dev, "Failed to set control update for %s\n",
236233
scontrol->name);
@@ -245,7 +242,7 @@ static void sof_ipc4_refresh_generic_control(struct snd_sof_control *scontrol)
245242
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
246243
struct snd_soc_component *scomp = scontrol->scomp;
247244
struct sof_ipc4_control_msg_payload *data;
248-
struct sof_ipc4_msg *msg = &cdata->msg;
245+
struct sof_ipc4_msg msg;
249246
size_t data_size;
250247
unsigned int i;
251248
int ret;
@@ -263,13 +260,13 @@ static void sof_ipc4_refresh_generic_control(struct snd_sof_control *scontrol)
263260

264261
data->id = cdata->index;
265262
data->num_elems = scontrol->num_channels;
266-
msg->data_ptr = data;
267-
msg->data_size = data_size;
263+
264+
memcpy(&msg, &cdata->msg, sizeof(msg));
265+
msg.data_ptr = data;
266+
msg.data_size = data_size;
268267

269268
scontrol->comp_data_dirty = false;
270-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, false, true);
271-
msg->data_ptr = NULL;
272-
msg->data_size = 0;
269+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, false, true);
273270
if (!ret) {
274271
for (i = 0; i < scontrol->num_channels; i++) {
275272
cdata->chanv[i].channel = data->chanv[i].channel;
@@ -291,7 +288,7 @@ sof_ipc4_set_bytes_control_data(struct snd_sof_control *scontrol, bool lock)
291288
struct snd_soc_component *scomp = scontrol->scomp;
292289
struct sof_ipc4_control_msg_payload *msg_data;
293290
struct sof_abi_hdr *data = cdata->data;
294-
struct sof_ipc4_msg *msg = &cdata->msg;
291+
struct sof_ipc4_msg msg;
295292
size_t data_size;
296293
int ret;
297294

@@ -304,14 +301,13 @@ sof_ipc4_set_bytes_control_data(struct snd_sof_control *scontrol, bool lock)
304301
msg_data->num_elems = data->size;
305302
memcpy(msg_data->data, data->data, data->size);
306303

307-
msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
304+
memcpy(&msg, &cdata->msg, sizeof(msg));
305+
msg.extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
308306

309-
msg->data_ptr = msg_data;
310-
msg->data_size = data_size;
307+
msg.data_ptr = msg_data;
308+
msg.data_size = data_size;
311309

312-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, true, lock);
313-
msg->data_ptr = NULL;
314-
msg->data_size = 0;
310+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, true, lock);
315311
if (ret < 0)
316312
dev_err(scomp->dev, "%s: Failed to set control update for %s\n",
317313
__func__, scontrol->name);
@@ -328,7 +324,7 @@ sof_ipc4_refresh_bytes_control(struct snd_sof_control *scontrol, bool lock)
328324
struct snd_soc_component *scomp = scontrol->scomp;
329325
struct sof_ipc4_control_msg_payload *msg_data;
330326
struct sof_abi_hdr *data = cdata->data;
331-
struct sof_ipc4_msg *msg = &cdata->msg;
327+
struct sof_ipc4_msg msg;
332328
size_t data_size;
333329
int ret = 0;
334330

@@ -346,39 +342,37 @@ sof_ipc4_refresh_bytes_control(struct snd_sof_control *scontrol, bool lock)
346342
if (!msg_data)
347343
return -ENOMEM;
348344

349-
msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
345+
memcpy(&msg, &cdata->msg, sizeof(msg));
346+
msg.extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
350347

351348
msg_data->id = cdata->index;
352349
msg_data->num_elems = 0; /* ignored for bytes */
353350

354-
msg->data_ptr = msg_data;
355-
msg->data_size = data_size;
351+
msg.data_ptr = msg_data;
352+
msg.data_size = data_size;
356353

357354
scontrol->comp_data_dirty = false;
358-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, false, lock);
355+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, false, lock);
359356
if (!ret) {
360-
if (msg->data_size > scontrol->max_size - sizeof(*data)) {
357+
if (msg.data_size > scontrol->max_size - sizeof(*data)) {
361358
dev_err(scomp->dev,
362359
"%s: no space for data in %s (%zu, %zu)\n",
363-
__func__, scontrol->name, msg->data_size,
360+
__func__, scontrol->name, msg.data_size,
364361
scontrol->max_size - sizeof(*data));
365362
ret = -EINVAL;
366363
goto out;
367364
}
368365

369-
data->size = msg->data_size;
366+
data->size = msg.data_size;
370367
scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
371-
memcpy(data->data, msg->data_ptr, data->size);
368+
memcpy(data->data, msg.data_ptr, data->size);
372369
} else {
373370
dev_err(scomp->dev, "Failed to read control data for %s\n",
374371
scontrol->name);
375372
scontrol->comp_data_dirty = true;
376373
}
377374

378375
out:
379-
msg->data_ptr = NULL;
380-
msg->data_size = 0;
381-
382376
kfree(msg_data);
383377

384378
return ret;
@@ -508,7 +502,7 @@ static int sof_ipc4_set_get_bytes_data(struct snd_sof_dev *sdev,
508502
{
509503
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
510504
struct sof_abi_hdr *data = cdata->data;
511-
struct sof_ipc4_msg *msg = &cdata->msg;
505+
struct sof_ipc4_msg msg;
512506
int ret = 0;
513507

514508
/* Send the new data to the firmware only if it is powered up */
@@ -530,28 +524,26 @@ static int sof_ipc4_set_get_bytes_data(struct snd_sof_dev *sdev,
530524
return sof_ipc4_refresh_bytes_control(scontrol, lock);
531525
}
532526

533-
msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
527+
memcpy(&msg, &cdata->msg, sizeof(msg));
528+
msg.extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);
534529

535-
msg->data_ptr = data->data;
530+
msg.data_ptr = data->data;
536531
if (set)
537-
msg->data_size = data->size;
532+
msg.data_size = data->size;
538533
else
539-
msg->data_size = scontrol->max_size - sizeof(*data);
534+
msg.data_size = scontrol->max_size - sizeof(*data);
540535

541-
ret = sof_ipc4_set_get_kcontrol_data(scontrol, set, lock);
536+
ret = sof_ipc4_set_get_kcontrol_data(scontrol, &msg, set, lock);
542537
if (ret < 0) {
543538
dev_err(sdev->dev, "Failed to %s for %s\n",
544539
set ? "set bytes update" : "get bytes",
545540
scontrol->name);
546541
} else if (!set) {
547542
/* Update the sizes according to the received payload data */
548-
data->size = msg->data_size;
543+
data->size = msg.data_size;
549544
scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
550545
}
551546

552-
msg->data_ptr = NULL;
553-
msg->data_size = 0;
554-
555547
return ret;
556548
}
557549

0 commit comments

Comments
 (0)