Repository navigation
Conversation
The compress-offload device is the only ALSA character device family which does not add its open files to the card's file list. Without it the core cannot replace the file operations on disconnect, so syscalls issued after the card is gone still reach the driver, and a card freed with snd_card_free_when_closed() does not wait for the open compress streams. Call snd_card_file_add() at open and snd_card_file_remove() at release, following the ordering used by the PCM, control, rawmidi and hwdep devices. Suggested-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Takashi Iwai <tiwai@suse.de>
The state of the stream is set to SNDRV_PCM_STATE_DRAINING only after the drain trigger has been sent to the driver. Drivers can not block in the trigger callback and the drain is completed asynchronously: snd_compr_drain_notify() can be called either from the trigger callback itself or from a different context as soon as the trigger returns. If that happens before the core updates the state then the notification is lost: the state is overwritten with DRAINING right after it and the wait for the drain to complete will not be woken up. Set the state before the trigger is issued and restore it if the trigger fails. The partial_drain flag is cleared on the error path as well as it would otherwise be left set for the next drain. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Two issues with the way sof_set_up_widgets_in_path() adds a pipeline to the list of pipelines to be triggered: The count is incremented before the array entry is written, so the last entry of the list is NULL for a short window. A concurrent user of the list, like sof_ipc4_trigger_pipelines(), can see the incremented count with the entry still unset. A widget which is not part of a pipeline has no spipe, but it is still added to the list, resulting in a NULL entry within the count. Store the entry before incrementing the count and skip the widgets without a pipeline. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The page table describing the audio buffer for the firmware is a single PAGE_SIZE allocation, but snd_sof_create_page_table() writes the compressed PFNs into it without checking that they fit. The PFNs are stored in 2.5 bytes each and the last one is written with a 32 bit access, so with 4K pages the table can hold 1638 PFNs, which is a buffer of 6.4MB. A larger buffer corrupts memory past the end of the page table. The PCM path is not affected as the maximum buffer size is specified by the topology, but the compressed streams can ask for larger buffers. Pass the page table buffer to snd_sof_create_page_table() instead of only its address and verify that the buffer can be described by it. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
snd_pcm_open() initializes the constraints of a PCM runtime to 'anything goes'. The runtime allocated for the compressed FE substream is zeroed, which means an empty mask and a [0, 0] interval instead, which the constraint helpers refine against and reject. Initialize the constraints the same way the PCM core does. The hw rules snd_pcm_hw_constraints_init() installs on top are only evaluated by snd_pcm_hw_refine(), which never runs for this substream, so they are not needed. This aligns the code with the upstream version of "ASoC: soc-compress: Provide a runtime for the compressed FE substream" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
SND_COMPR_TRIGGER_NEXT_TRACK, like the DRAIN and PARTIAL_DRAIN triggers, is a compress specific trigger which has no meaning for the BEs, it must only be passed to the FE component. This aligns the code with the upstream version of "ASoC: soc-compress: Implement trigger FE-BE sequencing as with normal PCMs" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The pipelines to be triggered are now only accessed with the pipeline_state_mutex held, which the PCM free also takes, and the pipeline_list no longer contains NULL entries or entries for widgets without a pipeline. The checks for NULL pipeline, pipe_widget and pipe_widget->private while walking the list are not needed, drop them. This aligns the code with the upstream version of "ASoC: SOF: ipc4-pcm: Serialize the PCM free with the pipeline triggers" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The sof_info data is coming from the firmware as part of the firmware config, it is guaranteed to be correct, no size validation is needed for the tuples. Simplify the parsing accordingly. This aligns the code with the upstream version of "ASoC: SOF: ipc4/ipc4-loader: Add SOF_INFO and CODEC_INFO to fw_config_params" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
…acity The compressed buffer must be describable by the page table allocated for the stream, otherwise the page table creation would fail. Limit the max_fragments reported in the capabilities so that a buffer made of max_fragments number of max_fragment_size fragments fits, and reject buffer sizes which can not be described by the page table. This aligns the code with the upstream version of "ASoC: SOF: Add support for IPC4 compressed" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The format of a SND_AUDIOCODEC_PCM stream is provided by user space via the codec parameters, it must be validated before it is used. Reject formats which are not valid PCM formats. This aligns the code with the upstream version of "ASoC: SOF: Add support for IPC4 compressed" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Gapless playback is not supported: the NEXT_TRACK trigger is already rejected, but the PARTIAL_DRAIN, which is only used for gapless playback, was handled as a normal DRAIN. Reject the PARTIAL_DRAIN trigger with -EOPNOTSUPP as well. This aligns the code with the upstream version of "ASoC: SOF: Add support for IPC4 compressed" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The host DMA is left running on stop and drain to match the firmware, which only pauses the pipeline, and it is only stopped during hw_free. A stream restarted after stop or drain - which the compress core allows for capture - would continue from a position the two sides no longer agree on. Track if the stream has been started and reject a subsequent START trigger. While at it, only clear the pending_stop flag for the triggers it is relevant to, a rejected trigger should not modify it. This aligns the code with the upstream version of "ASoC: SOF: Add support for IPC4 compressed" Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
ujfalusi
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
lyakh,
plbossart and
ranj063
as code owners
October 15, 2026 11:42
There was a problem hiding this comment.
🟡 Changes recommended
Removing nested firmware-tuple bounds checks permits out-of-bounds kernel reads from malformed replies.
1 open finding
What changed in this PR
Aligns ALSA/ASoC/SOF compressed-stream support with the upstream v5 implementation.
Changes:
- Adds page-table capacity checks and compressed-buffer limits.
- Updates stream triggers, constraints, and drain handling.
- Tracks compressed-device files and simplifies firmware tuple parsing.
| File | Description |
|---|---|
| sound/soc/sof/sof-utils.h | Defines page-table capacity and updates helper signature. |
| sound/soc/sof/sof-utils.c | Validates page-table capacity. |
| sound/soc/sof/sof-audio.h | Tracks compressed-stream start state. |
| sound/soc/sof/sof-audio.c | Filters pipeline entries and updates page-table calls. |
| sound/soc/sof/pcm.c | Passes the page-table buffer descriptor. |
| sound/soc/sof/ipc4-pcm.c | Removes pipeline null checks. |
| sound/soc/sof/ipc4-loader.c | Removes nested firmware-tuple bounds checks. |
| sound/soc/sof/ipc4-compress.c | Validates buffers, formats, and stream triggers. |
| sound/soc/sof/ipc3-dtrace.c | Updates trace page-table creation. |
| sound/soc/soc-compress.c | Initializes constraints and adjusts trigger routing. |
| sound/core/compress_offload.c | Adds card-file tracking and fixes drain-state ordering. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+428
to
+430
| * No size validation is needed as the sof_info data is coming from | ||
| * firmware as part of the firmware config, it is guarantied to be | ||
| * correct. |
Collaborator
|
Cherry picked the commits manually into the topic/sof-dev branch. |
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.
Cumulative patches on top of sof-dev to update the compress support code matching with what has finally got applied as v5