audio: fix NULL buffer-peer dereferences on IPC3 half-connected pipelines - #11060
Open
tmleman wants to merge 2 commits into
Open
audio: fix NULL buffer-peer dereferences on IPC3 half-connected pipelines#11060tmleman wants to merge 2 commits into
tmleman wants to merge 2 commits into
Conversation
tmleman
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
August 5, 2026 08:46
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
The functional changes correctly prevent the reported NULL dereferences, and the only remaining feedback is a minor comment wording fix.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Fixes two IPC3 fuzzer-found NULL-pointer dereferences that can occur when pipelines are built incrementally and buffers are only half-connected (one end still NULL). The changes harden graph walks against missing buffer peers while keeping behavior unchanged for valid/fully-connected topologies.
Changes:
selector_trigger(): guard a NULL source component before callingdev_comp_type(), and return immediately oncomp_set_state()errors.pipeline_get_dai_comp(): guardcomp(buffer peer) before dereferencingcomp->pipelineduring STREAM_POSITION-related traversal.
File summaries
| File | Description |
|---|---|
| src/audio/selector/selector.c | Avoids dereferencing a NULL producer component in selector_trigger() and properly propagates trigger/state errors. |
| src/audio/pipeline/pipeline-graph.c | Prevents NULL dereference by checking the buffer peer component before accessing comp->pipeline during DAI discovery. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The IPC fuzzer hit a NULL pointer dereference in the IPC3 path while triggering a pipeline that contains a selector component. Both AddressSanitizer and UBSan report a READ SEGV at address 0x30 in dev_comp_type() (src/include/sof/audio/component.h). 0x30 is the offset of comp_dev::ipc_config.type, i.e. the accessor was called on a NULL comp_dev. Root cause: a comp_buffer links a producer on its ->source side to a consumer on its ->sink side, and buffers are zero-initialised, so an unconnected side stays NULL. selector_trigger() checks only that its source buffer exists and then passes that buffer's producer component straight to dev_comp_type() to look for an upstream KPB: type = dev_comp_type(comp_buffer_get_source_component(sourceb)); When the source buffer is attached to the selector but never attached to an upstream producer, comp_buffer::source is NULL and dev_comp_type() dereferences it. The mirror-image lookup in kpb.c already guards against a missing peer component; selector_trigger() was the only site doing this without the check. Look up the producer component first and treat a missing producer as "not a KPB" instead of dereferencing it. Also honour the comp_set_state() return value: the crashing trigger was a PAUSE issued from COMP_STATE_READY, which comp_set_state() already rejects with -EINVAL, but selector_trigger() ignored the error and fell through to the dereference. Bail out on error like every other component trigger handler. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
The IPC fuzzer built an IPC3 pipeline whose component owned a half-connected buffer - present on the component's buffer list but with no component attached to its opposite end - and then issued a SOF_IPC_STREAM_POSITION request. ipc_stream_position() calls pipeline_get_timestamp(), which walks the graph with pipeline_get_dai_comp() to locate the DAI endpoint. For each hop the walk takes the buffer's peer component via buffer_get_comp() and immediately dereferences comp->pipeline. For a dangling buffer buffer_get_comp() returns NULL, so the read faults at NULL + 8 (the ->pipeline field). A comp_buffer only carries a peer on a side once both ends are attached, but the IPC3 graph is assembled incrementally (each COMP_CONNECT attaches a single end) so a buffer can be left half-connected. The STREAM_POSITION path reaches this walk directly, independent of the trigger path, and does not go through the pipeline-run connectivity checks: a pipeline can be completed and then have a half-connected buffer attached by a later COMP_CONNECT, and a subsequent POSITION request walks the dangling edge. Guard the peer before dereferencing it, mirroring the existing checks in the tree: the IPC4 sibling pipeline_get_dai_comp_latency() already checks "if (!source || !source->pipeline)", and the XRUN rewind walk in pipeline_trigger_xrun() checks "if (!buffer_comp || !buffer_comp->pipeline)". A fully-connected buffer always has a non-NULL peer and a cross-pipeline peer keeps a non-NULL pointer, so only a genuinely dangling buffer is rejected and valid topologies are unaffected. pipeline_get_timestamp() already tolerates a NULL DAI result. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
tmleman
requested review from
abonislawski,
lyakh,
serhiy-katsyuba-intel,
softwarecki and
wjablon1
August 5, 2026 12:33
serhiy-katsyuba-intel
approved these changes
Aug 5, 2026
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.
Two independent NULL-pointer dereferences found by the IPC3 fuzzer when a pipeline contains a half-connected buffer (one end attached to a component, the other end left NULL). The IPC3 graph is built incrementally (each COMP_CONNECT attaches a single buffer end) so a dangling edge can exist and later be walked.
dev_comp_type(). Missing producer caused a SEGV at the type accessor. Also now honours thecomp_set_state()return value instead of falling through on error.comp->pipelinewithout checking the buffer peer, faulting onNULL + 8.Guard the buffer peer before dereferencing it in both walks. Only a genuinely NULL peer is rejected; fully-connected and cross-pipeline peers are unaffected, so valid topologies keep working. Mirrors existing guards already in the tree (
kpb.c,pipeline_get_dai_comp_latency(),pipeline_trigger_xrun()).