Repository navigation
audio: up_down_mixer: validate host output channel map - #11392
Shrusti-pk wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch?
|
PR 11392: test resultsRun date: 2026-11-03 07:43 UTC Tested commit: 9cb761d7f9800d4299dd3fd57981b6848d1af597
|
| uint8_t left_surround_slot = get_channel_location(out_channel_map, CHANNEL_LEFT_SURROUND); | ||
| uint8_t right_surround_slot = get_channel_location(out_channel_map, CHANNEL_RIGHT_SURROUND); | ||
|
|
||
| /* Must support also 5.1 Surround */ | ||
| if (left_surround_slot == CHANNEL_INVALID && right_surround_slot == CHANNEL_INVALID) { | ||
| left_surround_slot = get_channel_location(out_channel_map, CHANNEL_LEFT_SIDE); | ||
| right_surround_slot = get_channel_location(out_channel_map, CHANNEL_RIGHT_SIDE); | ||
| } | ||
|
|
There was a problem hiding this comment.
Need to be careful in the processing functions as these need to be fast. We should always do any validation in the module configuration APIs as these have time to validate. Pls try and moves the checks to non time critical functions.
There was a problem hiding this comment.
Makes sense, moved it. init_mix() now resolves the surround pair once (surround slots, or the side pair when a 5.1 Surround map names that instead) and stores the two slots in the module private data, and the validator checks those stored slots. The mono to 5.1 routines and upmix32bit_quatro_to_5_1() just read cd->out_left_surround_slot / cd->out_right_surround_slot now, so there's no lookup or branch left in the mix path from this change. The other 5.1 routines still carry their pre-existing per-call fallback, I left those alone to keep the diff focused but they could move to the stored slots the same way if you want.
There was a problem hiding this comment.
🔵 Needs a closer look
The HiFi3 memory-safety and surround-routing changes lack mixer regression tests and require target or equivalent runtime validation for malformed maps and 5.1/7.1 conversions.
No open issues 🎉
What changed in this PR
This PR hardens the up/down mixer against out-of-bounds writes caused by invalid host-supplied output channel maps, while preserving support for 5.1 layouts that use side channels.
Changes:
- Reject invalid 5.1 and 7.1 output channel maps during mixer initialization.
- Add side-channel fallback to both mono-to-5.1 routines and correct the quatro-to-5.1 fallback to use the output map.
| File | Description |
|---|---|
src/audio/up_down_mixer/up_down_mixer.c |
Validate output channel maps before initializing the mixer. |
src/audio/up_down_mixer/up_down_mixer_hifi3.c |
Align 5.1 side-channel handling with the new validation. |
Review effort: Balanced
up_down_mixer_init() takes the channel_map word of the IPC4 init payload as is when coefficients_select is one of the *_WITH_CHANNEL_MAP values. The 5.1 and 7.1 output routines then place each output channel at get_channel_location(map, channel) * 4 bytes into the frame and write there for every frame without any further check. A channel the map does not name resolves to CHANNEL_INVALID (0xF) and a channel placed in nibble 6 or 7 of a 5.1 map resolves to a slot past the six channel frame, so the last frame is written past the end of the sink buffer. Add out_channel_map_valid() and call it from init_mix(). A 5.1 map must place left, center, right, LFE and either the surround pair or, when both surround channels are absent, the side pair at slots below 6. A 7.1 map must name all eight channels. Mono and stereo output routines do not use the output map. The default maps built by create_channel_map() pass unchanged. Resolve the 5.1 surround pair once in init_mix() as well: the slots the surround channels occupy, or the side pair when a 5.1 Surround map names that instead, are stored in the module private data so the processing routines read them without repeating the lookup. The two mono to 5.1 routines, which previously wrote the surround pair only, and upmix32bit_quatro_to_5_1(), which looked the side pair up in the input map, now use these slots, so the rule checked at init matches what every 5.1 routine writes and no check runs in the mix path. Signed-off-by: Shrushti P K <shrusthi@labs.digiscrypt.com>
9cb761d to
111ec98
Compare
|
test this please |
The up_down_mixer writes through a host supplied channel map without checking where it points:
Rejected such maps in init_mix(); the default maps from create_channel_map() and the 5.1 surround variant with the side pair still pass. The two mono to 5.1 routines gain the side pair fallback their siblings already have, and the quatro routine now reads the side pair from the output map, so the init check matches what every 5.1 routine writes.