Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The guard fixes the undefined shift while preserving behavior for smaller channel masks.
Review effort: Balanced
Findings: None
What changed in this PR
Prevents undefined 32-bit shifts when generating eight-channel ALH channel maps.
Changes:
- Skips absent-channel filling when all eight channels are present.
- Documents the avoided undefined behavior.
| File | Description |
|---|---|
src/audio/copier/copier_dai.c |
Safely generates the eight-channel identity map. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
abonislawski
reviewed
Oct 2, 2026
| nibble_map |= 0xFFFFFFFF << (channel_count * 4); | ||
| /* Absent channel is represented as 0xf nibble. With all 8 channels present the shift count | ||
| * would be 32, which is undefined behavior for 32-bit types. | ||
| * On Xtensa and x86 architectures this would result in returning 0xffffffff, marking all |
bitmask_to_nibble_channel_map() fills the nibbles of absent channels with 0xf using 0xFFFFFFFF << (channel_count * 4). channel_mask comes from the host-supplied ALH multi-gateway blob and popcount() == 8 is accepted by copier_set_alh_multi_gtw_channel_map(), so a mask of 0xff makes the shift count 32, which is undefined behaviour for a 32-bit type (UBSan: "shift exponent 32 is too large for 32-bit type"). Xtensa and x86 mask the shift count to 5 bits, so the shift by 32 behaves as a shift by 0 and the map becomes 0xFFFFFFFF: every channel of an 8-channel ALH aggregation is then marked absent in copier_dai_params() instead of getting the identity map 0x76543210. Only fill absent nibbles when there are fewer than 8 channels and use an unsigned literal. Results for all masks with fewer than 8 channels are unchanged. Found by the IPC4 libFuzzer campaign with -fsanitize=undefined. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bitmask_to_nibble_channel_map() fills the nibbles of absent channels with 0xf using 0xFFFFFFFF << (channel_count * 4). channel_mask comes from the host-supplied ALH multi-gateway blob and popcount() == 8 is accepted by copier_set_alh_multi_gtw_channel_map(), so a mask of 0xff makes the shift count 32, which is undefined behaviour for a 32-bit type (UBSan: "shift exponent 32 is too large for 32-bit type").
Xtensa and x86 mask the shift count to 5 bits, so the shift by 32 behaves as a shift by 0 and the map becomes 0xFFFFFFFF: every channel of an 8-channel ALH aggregation is then marked absent in copier_dai_params() instead of getting the identity map 0x76543210.
Only fill absent nibbles when there are fewer than 8 channels and use an unsigned literal. Results for all masks with fewer than 8 channels are unchanged.
Found by the IPC4 libFuzzer campaign with -fsanitize=undefined.