selector: re-validate configuration on control set by lgirdwood · Pull Request #10912 · thesofproject/sof

lgirdwood · 2026-06-15T09:45:20Z

The selector validates in/out channel counts at .params() time but accepts a
new configuration via the control set path afterwards without re-validating,
so a later out-of-range channel count could overflow the channel table.
Re-validate the blob size and channel fields in the control set handler
before accepting the new config.

No functional change for valid configurations.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Re-validates selector configuration received via the control-set (binary) path to prevent accepting invalid channel counts/selection at runtime (which could later overflow internal tables).

Changes:

Add minimum blob-size validation before parsing struct sof_sel_config.
Re-validate in_channels_count, out_channels_count, and sel_channel in selector_ctrl_set_data() before applying the config.

lgirdwood · 2026-06-15T10:12:59Z

+		if (cfg->sel_channel >= SEL_SOURCE_CHANNELS_MAX) {
+			comp_err(dev, "invalid sel_channel %u", cfg->sel_channel);
+			return -EINVAL;
+		}


Good point — added. sel_channel is now rejected when it is >= the configured in_channels_count (when that is non-zero/fixed), in addition to the absolute SEL_SOURCE_CHANNELS_MAX bound. When in_channels_count is 0 (derive from the stream at runtime) the absolute bound applies and .params() still checks against the actual stream channel count.

lgirdwood · 2026-06-15T10:13:01Z

+		switch (cfg->in_channels_count) {
+		case 0:
+		case SEL_SOURCE_2CH:
+		case SEL_SOURCE_4CH:
+			break;
+		default:
+			comp_err(dev, "invalid in_channels_count %u",
+				 cfg->in_channels_count);
+			return -EINVAL;
+		}
+
+		switch (cfg->out_channels_count) {
+		case 0:
+		case SEL_SINK_1CH:
+		case SEL_SINK_2CH:
+		case SEL_SINK_4CH:
+			break;
+		default:
+			comp_err(dev, "invalid out_channels_count %u",
+				 cfg->out_channels_count);
+			return -EINVAL;
+		}


0 isn't a "no change" sentinel here — it's a valid configuration meaning "derive the channel count from the stream at runtime". See selector_verify_params(): out_channels = cd->config.out_channels_count ? cd->config.out_channels_count : audio_stream_get_channels(...) (same for in). So accepting and storing 0 is correct; the effective count is resolved (and re-validated) at .params() time.

lgirdwood · 2026-06-15T10:13:02Z

 		/* Just set the configuration */
 		cd->config.in_channels_count = cfg->in_channels_count;
 		cd->config.out_channels_count = cfg->out_channels_count;


0 isn't a "no change" sentinel here — it's a valid configuration meaning "derive the channel count from the stream at runtime". See selector_verify_params(): out_channels = cd->config.out_channels_count ? cd->config.out_channels_count : audio_stream_get_channels(...) (same for in). So accepting and storing 0 is correct; the effective count is resolved (and re-validated) at .params() time.

lgirdwood · 2026-06-15T10:13:04Z

+		switch (cfg->in_channels_count) {
+		case 0:
+		case SEL_SOURCE_2CH:
+		case SEL_SOURCE_4CH:
+			break;
+		default:
+			comp_err(dev, "invalid in_channels_count %u",
+				 cfg->in_channels_count);
+			return -EINVAL;
+		}
+
+		switch (cfg->out_channels_count) {
+		case 0:
+		case SEL_SINK_1CH:
+		case SEL_SINK_2CH:
+		case SEL_SINK_4CH:
+			break;
+		default:
+			comp_err(dev, "invalid out_channels_count %u",
+				 cfg->out_channels_count);
+			return -EINVAL;
+		}


The two paths validate different things, which is why I didn't share code: selector_verify_params() validates the effective in/out channel counts at .params() time — deriving them from the stream when the config value is 0 and checking sel_channel against the actual runtime channel count. This handler validates the raw config fields before they're stored (accepted values, and sel_channel against the fixed input count). A shared helper would only cover the accepted-value switches and would mean restructuring verify_params to validate raw values before deriving. Happy to extract one if you'd prefer that, but kept them separate to avoid changing the prepare-time logic.

singalsu · 2026-06-15T12:42:55Z

+		 */
+		switch (cfg->in_channels_count) {
+		case 0:
+		case SEL_SOURCE_2CH:


We have also 5.1 and 7.1 down-mix supported. See https://github.com/thesofproject/sof/tree/main/tools/topology/topology2/include/components/micsel

and also 1CH input is supported

singalsu · 2026-06-15T12:43:29Z

+		case SEL_SINK_2CH:
+		case SEL_SINK_4CH:
+			break;
+		default:


also 5.1 and 7.1 sinks

The accepted set here intentionally mirrors the existing selector_verify_params() (.params() path), which only allows SEL_SINK_1CH/2CH/4CH today — so this re-validation doesn't change what the component accepts. Adding 5.1 (6) and 7.1 (8) would be a feature change: it needs the same cases added to selector_verify_params() and the passthrough check, plus confirming the selector processing handles 6/8 ch. Happy to do it, but it feels like a separate enhancement rather than part of this security re-validation fix — would you prefer it here or as a follow-up?

lgirdwood · 2026-06-18T16:13:04Z

Fixed the build failures: I'd used SEL_SOURCE_CHANNELS_MAX in the IPC3 selector_ctrl_set_data(), but that macro is only defined in the IPC4 (#else) branch of selector.h — so the IPC3/native/cmocka builds failed with 'undeclared'. Switched to SEL_SOURCE_4CH (the IPC3 max source channel count). Pushed.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

lgirdwood · 2026-06-19T14:05:27Z

+		/* sel_channel indexes the source channels: bound it by the
+		 * maximum supported source channel count, and (when the input
+		 * count is fixed, i.e. non-zero) by the configured input count
+		 */
+		if (cfg->sel_channel >= SEL_SOURCE_4CH ||
+		    (cfg->in_channels_count &&
+		     cfg->sel_channel >= cfg->in_channels_count)) {
+			comp_err(dev, "invalid sel_channel %u (in_channels_count %u)",
+				 cfg->sel_channel, cfg->in_channels_count);
+			return -EINVAL;
+		}


Addressed. The control set path now validates sel_channel against the live source channel count when in_channels_count == 0: it fetches the connected source producer (comp_dev_get_first_data_producer) and bounds sel_channel by that stream's channel count, mirroring the .params() check at line 142. If the source is not yet connected when the blob arrives, it falls back to the static SEL_SOURCE_4CH bound, and .params() still re-checks against the negotiated channel count before streaming. So a late control update setting sel_channel=3 on a 2-channel stream is now rejected rather than reaching the copy routine.

lgirdwood · 2026-06-19T14:05:13Z

+		/*
+		 * The config validated at .params() time can be replaced here at
+		 * runtime, so re-validate the new channel counts and selected
+		 * channel before accepting them; otherwise an out-of-range value
+		 * later overflows the channel coefficient table during process().
+		 */


Good catch — there is no coefficient table in the IPC3 selector. Reworded the comment to state the real consequence: an out-of-range value later indexes past the source channels in the copy routine.

A new configuration accepted at runtime through the control set path was copied in without the validation done when parameters are applied, so a later out-of-range channel count could overflow the channel table. Validate the blob size and channel fields before accepting it. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

+		/* sel_channel indexes the source channels, so it must be below
+		 * the source channel count, otherwise the copy routine reads
+		 * past the end of each source frame. Bound it by the maximum
+		 * supported source channel count, and by the actual input count:
+		 * the configured value when fixed (non-zero), or the live source
+		 * stream channel count when the input count varies (zero) and the
+		 * source is already connected.
+		 */
+		live_channels = cfg->in_channels_count;
+		if (!live_channels) {
+			struct comp_buffer *src =
+				comp_dev_get_first_data_producer(dev);
+
+			if (src)
+				live_channels =
+					audio_stream_get_channels(&src->stream);
+		}
+
+		if (cfg->sel_channel >= SEL_SOURCE_4CH ||
+		    (live_channels && cfg->sel_channel >= live_channels)) {
+			comp_err(dev, "invalid sel_channel %u (in_channels_count %u)",
+				 cfg->sel_channel, cfg->in_channels_count);
+			return -EINVAL;
+		}


Copilot AI review requested due to automatic review settings June 15, 2026 09:45

lgirdwood requested a review from tlissows as a code owner June 15, 2026 09:45

Copilot AI reviewed Jun 15, 2026

View reviewed changes

lgirdwood force-pushed the fix-selector branch from aad0658 to c4275a8 Compare June 15, 2026 10:12

Copilot started reviewing on behalf of lgirdwood June 15, 2026 10:32 View session

singalsu requested changes Jun 15, 2026

View reviewed changes

lgirdwood force-pushed the fix-selector branch from c4275a8 to 037a9cf Compare June 18, 2026 16:08

lgirdwood requested a review from Copilot June 19, 2026 11:55

Copilot started reviewing on behalf of lgirdwood June 19, 2026 11:55 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

lgirdwood force-pushed the fix-selector branch from 037a9cf to cceaf11 Compare June 19, 2026 14:04

lgirdwood requested a review from Copilot June 19, 2026 16:10

Copilot started reviewing on behalf of lgirdwood June 19, 2026 16:11 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Conversation

lgirdwood commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants