audio: drc: register IPC-time blob validator#10943
Conversation
singalsu
force-pushed
the
model_handler_validator_drc
branch
from
20c85b6 to
e0ff5f6
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds an IPC-time validator for the DRC configuration blob so malformed run-time updates are rejected before they can replace the currently active configuration, and reuses the same validation at prepare() time when initially loading the blob.
Changes:
- Introduce a
drc_validator()that enforces exact payload size matchingstruct sof_drc_configand verifiesconfig->sizeconsistency. - Apply the validator to the initial configuration fetch in
drc_prepare(). - Register/unregister the validator on the data-blob handler in
drc_prepare()/drc_reset()to prevent corrupted IPC updates from being accepted during streaming.
singalsu
force-pushed
the
model_handler_validator_drc
branch
from
e0ff5f6 to
d282b64
Compare
Hook a drc blob validator into the model handler so a corrupted run-time configuration update is rejected before it can replace the working blob. Playback or capture then continues with the previously set parameters instead of being interrupted by a bad IPC. The DRC configuration is a fixed-size struct sof_drc_config, so the validator requires the IPC payload size to match exactly and the self-declared config->size to agree with it. It is installed in drc_init() when the model handler is created, so every blob swap is covered - including any received while the component is in READY - and the redundant size check that drc_prepare() used to run on the initial blob is dropped. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
singalsu
force-pushed
the
model_handler_validator_drc
branch
from
d282b64 to
3f08b13
Compare
| static int drc_validator(struct comp_dev *dev, void *new_data, uint32_t new_data_size) | ||
| { | ||
| struct sof_drc_config *config = new_data; | ||
|
|
||
| if (new_data_size != sizeof(struct sof_drc_config) || | ||
| new_data_size != config->size) { | ||
| comp_err(dev, "invalid configuration blob, size %u, expected %zu", | ||
| new_data_size, sizeof(struct sof_drc_config)); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
I asked an expert: "Not valid as stated — the code is already safe thanks to C's || short-circuit. If new_data_size < sizeof(struct sof_drc_config) (the reviewer's "size < 4" case), the first sub-expression is true and evaluation stops right there — config->size is never dereferenced. config->size is only read when new_data_size == sizeof(struct sof_drc_config), i.e. when the payload is guaranteed to hold a full sof_drc_config. So there is no out-of-bounds read."
So, I'll keep this as-is.
| cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL); | ||
| /* the blob is dereferenced as a struct sof_drc_config below and in | ||
| * drc_setup(), so require it to be at least that large | ||
| */ | ||
| if (cd->config && data_size >= sizeof(struct sof_drc_config)) { | ||
|
|
||
| if (cd->config) { |
There was a problem hiding this comment.
My expert buddy's response: "The commit body was updated in the last push and no longer says "reused at prepare time" — that sentence was removed together with the explicit validator call. An extra validator call in drc_prepare() would be dead code because the handler is created empty in drc_init() and the validator is installed on the very next line, before any comp_data_blob_set() can run. There is no path through which an unvalidated blob can reach cd->model_handler, so comp_get_data_blob() in prepare can only return NULL or a previously validated pointer."
Hook a drc blob validator into the model handler so a corrupted run-time configuration update is rejected before it can replace the working blob. Playback or capture then continues with the previously set parameters instead of being interrupted by a bad IPC.
The DRC configuration is a fixed-size struct sof_drc_config, so the validator requires the IPC payload size to match exactly and the self-declared config->size to agree with it. The same size check is also reused at prepare time when the initial blob is fetched.