rtnr: clamp host-supplied control element counts#10911
Conversation
lgirdwood
requested review from
dbaluta,
kv2019i,
lbetlej,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds bounds-checking for host-supplied SWITCH control element counts in rtnr get/set handlers to prevent reading/writing past the IPC payload.
Changes:
- Validate
cdata->num_elemsagainstSOF_IPC_MAX_CHANNELSinrtnr_get_config(). - Validate
cdata->num_elemsagainstSOF_IPC_MAX_CHANNELSinrtnr_set_value().
| if (cdata->num_elems > SOF_IPC_MAX_CHANNELS) { | ||
| comp_err(dev, "rtnr_get_config() error: num_elems %u out of range", | ||
| cdata->num_elems); | ||
| return -EINVAL; | ||
| } |
There was a problem hiding this comment.
Reworded — the behaviour is intentional rejection (a control message with more elements than channels is malformed). I consolidated the get/set changes into a single commit titled "rtnr: reject out-of-range control switch element counts" and updated the PR description to match.
| if (cdata->num_elems > SOF_IPC_MAX_CHANNELS) { | ||
| comp_err(dev, "rtnr_set_value() error: num_elems %u out of range", | ||
| cdata->num_elems); | ||
| return -EINVAL; | ||
| } |
There was a problem hiding this comment.
Fixed — both messages now include the limit: num_elems %u > max %u (with SOF_IPC_MAX_CHANNELS).
The SWITCH control get and set handlers looped over a host-supplied element count while reading/writing the control channel array, which could run past the IPC payload. Reject counts larger than the maximum channel count in both handlers before the loop. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
abonislawski
left a comment
abonislawski
left a comment
There was a problem hiding this comment.
@lgirdwood looks like there are 2 accidentally added txt files?
|
|
||
| case SOF_CTRL_CMD_SWITCH: | ||
| if (cdata->num_elems > SOF_IPC_MAX_CHANNELS) { | ||
| comp_err(dev, "rtnr_get_config() error: num_elems %u > max %u", |
There was a problem hiding this comment.
"rtnr_get_config()" func name duplicate not needed
| int32_t j; | ||
|
|
||
| if (cdata->num_elems > SOF_IPC_MAX_CHANNELS) { | ||
| comp_err(dev, "rtnr_set_value() error: num_elems %u > max %u", |
There was a problem hiding this comment.
"rtnr_set_value()" func name duplicate not needed
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
The alsa-info.txt and dump.txt look like accidentally included to this PR.
The SWITCH control get and set handlers looped over a host-supplied element
count while reading/writing the control channel array, which could run past
the IPC payload. Reject counts larger than
SOF_IPC_MAX_CHANNELSin bothhandlers before the loop.
No functional change for valid configurations.