Skip to content

Commit 960976a

Browse files
lyakhranj063
authored andcommitted
audio: don't rely on correct IPC order
If the host sends IPCs to the DSP in wrong order, .prepare() can be called without correctly initialised buffers. Add checks everywhere where this can cause NULL dereference. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
1 parent 22fe8e6 commit 960976a

29 files changed

Lines changed: 150 additions & 25 deletions

File tree

src/audio/aria/aria.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,13 @@ static int aria_prepare(struct processing_module *mod,
190190
comp_info(dev, "aria_prepare()");
191191

192192
source = comp_dev_get_first_data_producer(dev);
193-
aria_set_stream_params(source, mod);
194-
195193
sink = comp_dev_get_first_data_consumer(dev);
194+
if (!source || !sink) {
195+
comp_err(dev, "no source or sink buffer");
196+
return -ENOTCONN;
197+
}
198+
199+
aria_set_stream_params(source, mod);
196200
aria_set_stream_params(sink, mod);
197201

198202
if (audio_stream_get_valid_fmt(&source->stream) != SOF_IPC_FRAME_S24_4LE ||

src/audio/asrc/asrc.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,10 @@ static int asrc_params(struct processing_module *mod)
399399

400400
sourceb = comp_dev_get_first_data_producer(dev);
401401
sinkb = comp_dev_get_first_data_consumer(dev);
402+
if (!sourceb || !sinkb) {
403+
comp_err(dev, "no source or sink buffer");
404+
return -ENOTCONN;
405+
}
402406

403407
/* update the source/sink buffer formats. Sink rate will be modified below */
404408
asrc_update_buffer_format(sourceb, cd);
@@ -549,7 +553,10 @@ static int asrc_prepare(struct processing_module *mod,
549553
if (ret < 0)
550554
return ret;
551555

552-
/* SRC component will only ever have 1 source and 1 sink buffer */
556+
/*
557+
* SRC component will only ever have 1 source and 1 sink buffer,
558+
* asrc_params() has checked their validity already
559+
*/
553560
sourceb = comp_dev_get_first_data_producer(dev);
554561
sinkb = comp_dev_get_first_data_consumer(dev);
555562

src/audio/crossover/crossover.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,11 +534,16 @@ static int crossover_prepare(struct processing_module *mod,
534534

535535
comp_info(dev, "crossover_prepare()");
536536

537+
source = comp_dev_get_first_data_producer(dev);
538+
if (!source) {
539+
comp_err(dev, "no source buffer");
540+
return -ENOTCONN;
541+
}
542+
537543
crossover_params(mod);
538544

539545
/* Crossover has a variable number of sinks */
540546
mod->max_sinks = SOF_CROSSOVER_MAX_STREAMS;
541-
source = comp_dev_get_first_data_producer(dev);
542547

543548
/* Get source data format */
544549
cd->source_format = audio_stream_get_frm_fmt(&source->stream);

src/audio/crossover/crossover_ipc4.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ void crossover_params(struct processing_module *mod)
118118
ipc4_base_module_cfg_to_stream_params(&mod->priv.cfg.base_cfg, params);
119119
component_set_nearest_period_frames(dev, params->rate);
120120

121+
/* First producer verified by the caller */
121122
sourceb = comp_dev_get_first_data_producer(dev);
122123
ipc4_update_buffer_format(sourceb, &mod->priv.cfg.base_cfg.audio_fmt);
123124

src/audio/dcblock/dcblock.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,15 @@ static int dcblock_prepare(struct processing_module *mod,
195195

196196
comp_info(dev, "dcblock_prepare()");
197197

198-
dcblock_params(mod);
199-
200198
/* DC Filter component will only ever have one source and sink buffer */
201199
sourceb = comp_dev_get_first_data_producer(dev);
202200
sinkb = comp_dev_get_first_data_consumer(dev);
201+
if (!sourceb || !sinkb) {
202+
comp_err(dev, "no source or sink buffer");
203+
return -ENOTCONN;
204+
}
205+
206+
dcblock_params(mod);
203207

204208
/* get source data format */
205209
cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream);

src/audio/dcblock/dcblock_ipc4.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ void dcblock_params(struct processing_module *mod)
6161
ipc4_base_module_cfg_to_stream_params(&mod->priv.cfg.base_cfg, params);
6262
component_set_nearest_period_frames(dev, params->rate);
6363

64+
/* The caller has verified, that sink and source buffers are connected */
65+
6466
sinkb = comp_dev_get_first_data_consumer(dev);
6567
ipc4_update_buffer_format(sinkb, &mod->priv.cfg.base_cfg.audio_fmt);
6668

src/audio/drc/drc.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,8 @@ static void drc_params(struct processing_module *mod)
326326
ipc4_base_module_cfg_to_stream_params(&mod->priv.cfg.base_cfg, params);
327327
component_set_nearest_period_frames(dev, params->rate);
328328

329+
/* The caller has verified, that sink and source buffers are connected */
330+
329331
sinkb = comp_dev_get_first_data_consumer(dev);
330332
ipc4_update_buffer_format(sinkb, &mod->priv.cfg.base_cfg.audio_fmt);
331333

@@ -347,13 +349,17 @@ static int drc_prepare(struct processing_module *mod,
347349

348350
comp_info(dev, "drc_prepare()");
349351

350-
#if CONFIG_IPC_MAJOR_4
351-
drc_params(mod);
352-
#endif
353-
354352
/* DRC component will only ever have 1 source and 1 sink buffer */
355353
sourceb = comp_dev_get_first_data_producer(dev);
356354
sinkb = comp_dev_get_first_data_consumer(dev);
355+
if (!sourceb || !sinkb) {
356+
comp_err(dev, "no source or sink buffer");
357+
return -ENOTCONN;
358+
}
359+
360+
#if CONFIG_IPC_MAJOR_4
361+
drc_params(mod);
362+
#endif
357363

358364
/* get source data format */
359365
cd->source_format = audio_stream_get_frm_fmt(&sourceb->stream);

src/audio/eq_fir/eq_fir.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -416,15 +416,20 @@ static int eq_fir_prepare(struct processing_module *mod,
416416

417417
comp_dbg(dev, "eq_fir_prepare()");
418418

419+
/* EQ component will only ever have 1 source and 1 sink buffer. */
420+
sourceb = comp_dev_get_first_data_producer(dev);
421+
sinkb = comp_dev_get_first_data_consumer(dev);
422+
if (!sourceb || !sinkb) {
423+
comp_err(dev, "no source or sink buffer");
424+
return -ENOTCONN;
425+
}
426+
419427
ret = eq_fir_params(mod);
420428
if (ret < 0) {
421429
comp_set_state(dev, COMP_TRIGGER_RESET);
422430
return ret;
423431
}
424432

425-
/* EQ component will only ever have 1 source and 1 sink buffer. */
426-
sourceb = comp_dev_get_first_data_producer(dev);
427-
sinkb = comp_dev_get_first_data_consumer(dev);
428433
eq_fir_set_alignment(&sourceb->stream, &sinkb->stream);
429434
channels = audio_stream_get_channels(&sinkb->stream);
430435
frame_fmt = audio_stream_get_frm_fmt(&sourceb->stream);

src/audio/eq_fir/eq_fir_ipc4.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ int eq_fir_params(struct processing_module *mod)
5959
ipc4_base_module_cfg_to_stream_params(&mod->priv.cfg.base_cfg, params);
6060
component_set_nearest_period_frames(dev, params->rate);
6161

62+
/* The caller has verified, that sink and source buffers are connected */
63+
6264
sourceb = comp_dev_get_first_data_producer(dev);
6365
ipc4_update_buffer_format(sourceb, &mod->priv.cfg.base_cfg.audio_fmt);
6466

src/audio/eq_iir/eq_iir.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,18 @@ static int eq_iir_prepare(struct processing_module *mod,
187187

188188
comp_dbg(dev, "eq_iir_prepare()");
189189

190+
/* EQ component will only ever have 1 source and 1 sink buffer */
191+
sourceb = comp_dev_get_first_data_producer(dev);
192+
sinkb = comp_dev_get_first_data_consumer(dev);
193+
if (!sourceb || !sinkb) {
194+
comp_err(dev, "no source or sink buffer");
195+
return -ENOTCONN;
196+
}
197+
190198
ret = eq_iir_prepare_sub(mod);
191199
if (ret < 0)
192200
return ret;
193201

194-
/* EQ component will only ever have 1 source and 1 sink buffer */
195-
sourceb = comp_dev_get_first_data_producer(dev);
196-
sinkb = comp_dev_get_first_data_consumer(dev);
197202
eq_iir_set_alignment(&sourceb->stream, &sinkb->stream);
198203

199204
/* get source and sink data format */

0 commit comments

Comments
 (0)