audio: check buffers before calling .prepare()

[lyakh]

Similarly to #9985, .prepare() can also cause NULL dereference in case of wrong IPC order

[marcinszkudlinski]

the only place in the code calling .prepare() is the helper below. Why don't check params there?

int module_prepare(struct processing_module *mod,
		   struct sof_source **sources, int num_of_sources,
		   struct sof_sink **sinks, int num_of_sinks)
{
	int ret = 0;
	struct module_data *md = &mod->priv;
	struct comp_dev *dev = mod->dev;
	const struct module_interface *const ops = dev->drv->adapter_ops;

	comp_dbg(dev, "module_prepare() start");

#if CONFIG_IPC_MAJOR_3
	if (mod->priv.state == MODULE_IDLE)
		return 0;
	if (mod->priv.state < MODULE_INITIALIZED)
		return -EPERM;
#endif
	if (ops->prepare) {
		ret = ops->prepare(mod, sources, num_of_sources, sinks, num_of_sinks);

[lyakh]

the only place in the code calling .prepare() is the helper below. Why don't check params there?

@marcinszkudlinski because not every module uses them, AFAICS. Some use only one - either sources or sinks, some always run the list, so they should detect, when lists are empty

[Copilot]

Pull Request Overview

This PR adds buffer validations in various audio components to prevent NULL dereferences during the preparation phase, similar to the fix applied in #9985. Key changes include:

• Adding explicit null-checks for source and/or sink buffers before calling .prepare().
• Refactoring error-handling code in several modules to remove redundant local variables.
• Minor cleanup in warning messages and comments.

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/audio/multiband_drc/multiband_drc.c	Adds a null-check for the source buffer.
src/audio/module_adapter/module_adapter_ipc3.c	Removes redundant declaration of a variable for clarity.
src/audio/module_adapter/module/waves/waves.c	Adds a null-check for source and sink buffers.
src/audio/module_adapter/module/generic.c	Refactors error handling and removes a redundant variable.
src/audio/mixer/mixer.c	Adds a null-check for the sink buffer.
src/audio/mfcc/mfcc.c	Adds a null-check for both source and sink buffers.
src/audio/igo_nr/igo_nr.c	Adds a null-check for source and sink buffers.
src/audio/google/google_ctc_audio_processing.c	Adds a null-check for the source buffer.
src/audio/eq_iir/eq_iir_ipc4.c	Removes redundant variable and simplifies parameter setting.
src/audio/eq_iir/eq_iir_ipc3.c	Adds a clarifying comment about buffer verification.
src/audio/eq_iir/eq_iir.c	Adds null-checks for buffers and moves the alignment call.
src/audio/eq_fir/eq_fir_ipc4.c	Updates comments to note verified buffers.
src/audio/eq_fir/eq_fir.c	Adds a null-check for buffers and cleans up redundant code.
src/audio/drc/drc.c	Adds null-checks and repositions the drc_params call.
src/audio/dcblock/dcblock_ipc4.c	Adds a comment regarding buffer verification.
src/audio/dcblock/dcblock.c	Adds a null-check for buffers before updating parameters.
src/audio/crossover/crossover_ipc4.c	Adds a comment highlighting buffer verification.
src/audio/crossover/crossover.c	Adds null-checks and refactors error handling in setup.
src/audio/asrc/asrc.c	Adds null-checks and updates comments in buffer handling.
src/audio/aria/aria.c	Adds a null-check for buffers before setting stream params.

Comments suppressed due to low confidence (2)

src/audio/module_adapter/module/generic.c:193

• The new declaration of 'ret' here shadows any previous declaration. Consider renaming this local variable or reusing the outer variable for clarity.

int ret = ops->prepare(mod, sources, num_of_sources, sinks, num_of_sinks);

src/audio/crossover/crossover.c:574

• This local declaration of 'ret' shadows an earlier 'ret' variable. Renaming it or reusing the previous one may help improve code clarity.

int ret = crossover_setup(mod, channels);

[ranj063]

@lyakh this is awesome and allow us to prevent all sorts of crashes if the IPC ordering is wrong but can we take this one step further (in a future PR of course) and make sure params() and prepare() work with out of order widget setup and connections?

[lyakh]

@lyakh this is awesome and allow us to prevent all sorts of crashes if the IPC ordering is wrong but can we take this one step further (in a future PR of course) and make sure params() and prepare() work with out of order widget setup and connections?

@ranj063 don't know, is it possible and desirable? I.e. should it be possible to prepare a pipeline before it's connected with other pipelines into a contiguous stream? I thought it wasn't by chance that so many components use both source and sink buffers during configuration, and if the pipeline isn't connected into a stream, at least one of those buffers will be missing? And even if we do decide, that we have to support that, supposedly this should be done by someone, who can test such flows

[lgirdwood]

@lyakh this is awesome and allow us to prevent all sorts of crashes if the IPC ordering is wrong but can we take this one step further (in a future PR of course) and make sure params() and prepare() work with out of order widget setup and connections?

@ranj063 don't know, is it possible and desirable? I.e. should it be possible to prepare a pipeline before it's connected with other pipelines into a contiguous stream? I thought it wasn't by chance that so many components use both source and sink buffers during configuration, and if the pipeline isn't connected into a stream, at least one of those buffers will be missing? And even if we do decide, that we have to support that, supposedly this should be done by someone, who can test such flows

Can we use the pipeline state machine to detect and return errors on any out of order IPC ? We did this on IPC3, not sure if fully checked with IPC4 as driver does enforce this, but FW should too.

[lgirdwood]

LGTM, any followup work with state machine checking could be done via another PR

[ranj063]

@lyakh this is awesome and allow us to prevent all sorts of crashes if the IPC ordering is wrong but can we take this one step further (in a future PR of course) and make sure params() and prepare() work with out of order widget setup and connections?

@ranj063 don't know, is it possible and desirable? I.e. should it be possible to prepare a pipeline before it's connected with other pipelines into a contiguous stream? I thought it wasn't by chance that so many components use both source and sink buffers during configuration, and if the pipeline isn't connected into a stream, at least one of those buffers will be missing? And even if we do decide, that we have to support that, supposedly this should be done by someone, who can test such flows

Can we use the pipeline state machine to detect and return errors on any out of order IPC ? We did this on IPC3, not sure if fully checked with IPC4 as driver does enforce this, but FW should too.

@lgirdwood IPC3 had the PIPELINE_COMPLETE IPC where we did this, IPC4 doesnt have this and we call pipeline_complete() during start trigger. But my point @lyakh is that with IPC3 we propagated the params during the HW_PARAMS IPC in the firmware so that widgets could be ready before trigger. But in the case of IPC4, we get all the params during module init IPC, so we shouldnt need to do much during pipeline_params() at all

[lgirdwood]

SOFCI TEST

[kv2019i]

Please continue discussion on follow-up actions, I'll merge this as cutoff for 2.13 is imminent.