drc: validate host config blob sizes (dcblock, drc, multiband_drc)#10930
Conversation
The coefficient copy always read a fixed number of bytes from the config blob regardless of its actual size, over-reading adjacent heap for a short blob. Fall back to passthrough unless the blob holds the whole coefficient array. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
DRC setup dereferenced the config blob as a fixed struct without verifying the blob was at least that large, over-reading adjacent heap for a short blob. Require the blob to cover the config struct. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Setup read a base config struct and per-band coefficients from the blob without a size check, over-reading for a short blob. Require the blob to cover the base struct and num_bands band entries. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Hardens DRC-family modules against undersized host config blobs to prevent out-of-bounds reads during setup/config parsing.
Changes:
- Add minimum-size validation for
drcconfig blobs (must coverstruct sof_drc_config). - Add minimum-size validation for
dcblockcoefficient blobs (must cover the coefficient array before copying). - Add base+per-band payload validation for
multiband_drcblobs, including anum_bandsupper bound.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/audio/multiband_drc/multiband_drc.c | Validates blob size for base config + num_bands coefficient payload before setup. |
| src/audio/drc/drc.c | Ensures blob is at least sizeof(struct sof_drc_config) before dereferencing/configuring. |
| src/audio/dcblock/dcblock.c | Ensures blob is large enough for coefficient copy; otherwise switches to passthrough. |
| /* the blob holds a base struct followed by num_bands variable-length | ||
| * band coefficients; require the base struct first, then the full | ||
| * per-band payload, so setup cannot read past the blob | ||
| */ | ||
| if (cd->config && data_size >= sizeof(struct sof_multiband_drc_config) && | ||
| cd->config->num_bands <= SOF_MULTIBAND_DRC_MAX_BANDS && | ||
| data_size >= sizeof(struct sof_multiband_drc_config) + | ||
| (size_t)cd->config->num_bands * sizeof(struct sof_drc_params)) { |
There was a problem hiding this comment.
num_bands is bounded to SOF_MULTIBAND_DRC_MAX_BANDS before the multiplication (the && short-circuits left-to-right), so num_bands * sizeof(band) is at most MAX_BANDS * a small struct — it can't overflow size_t. The first term also requires data_size >= sizeof(base) before num_bands is even read.
| if (cd->config && data_size >= sizeof(struct sof_multiband_drc_config) && | ||
| cd->config->num_bands <= SOF_MULTIBAND_DRC_MAX_BANDS && | ||
| data_size >= sizeof(struct sof_multiband_drc_config) + | ||
| (size_t)cd->config->num_bands * sizeof(struct sof_drc_params)) { | ||
| ret = multiband_drc_setup(mod, channels, rate); |
There was a problem hiding this comment.
This matches the component's existing behaviour: an absent/!enabled config leaves it in passthrough rather than failing prepare(), so a too-small blob is treated the same way (skip setup, run passthrough). I kept that to avoid changing the failure semantics, but I'm happy to add a comp_warn() so a malformed blob is visible in the logs if you'd prefer.
| /* 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)) { | ||
| ret = drc_setup(mod, channels, rate); |
There was a problem hiding this comment.
Same as multiband_drc: the existing code already falls back to pass-through (drc_default_pass) when there's no usable config, so an undersized blob takes that same safe path rather than failing prepare(). Can add a comp_warn() for diagnosability if you'd like.
| if (cd->config && data_size >= sizeof(cd->R_coeffs)) | ||
| dcblock_copy_coefficients(mod); | ||
| else | ||
| dcblock_set_passthrough(mod); |
There was a problem hiding this comment.
Agreed it's safe but silent — I left it matching the existing passthrough fallback, but a comp_warn() when the blob is non-NULL yet too small is a reasonable addition. Happy to add it if you want it in this PR.
Hardening of the DRC-family components against host config blobs that are
smaller than the structures read out of them (out-of-bounds reads of
adjacent heap otherwise):
sizeof(struct sof_drc_config)num_bandsper-band coefficient entriesNo functional change for valid configurations.