Skip to content

pipeline: reject double-connect of already-attached buffer#10812

Open
tmleman wants to merge 1 commit into
thesofproject:mainfrom
tmleman:topic/upstream/pr/fix/ipc3/pipeline/reject_double_connect
Open

pipeline: reject double-connect of already-attached buffer#10812
tmleman wants to merge 1 commit into
thesofproject:mainfrom
tmleman:topic/upstream/pr/fix/ipc3/pipeline/reject_double_connect

Conversation

@tmleman
Copy link
Copy Markdown
Contributor

@tmleman tmleman commented May 26, 2026

pipeline_connect() had no guard against being called twice for the same buffer-component pair. Calling list_item_prepend() on a node that is already in a doubly-linked list corrupts the list by creating a self-loop where node->next points back to itself instead of to the list head.

The corruption was discovered through IPC3 fuzzing in persistent mode. Without per-testcase topology teardown, components and buffers created by testcase N survive into testcase N+1. When N+1 sends a TPLG_COMP_CONNECT for IDs that N already connected, ipc_comp_connect() finds the surviving objects and calls pipeline_connect() a second time. The same sequence can also be triggered within a single testcase by two CONNECT messages for the same pair.

The self-loop causes ipc_comp_free() to hang indefinitely. The function walks bsource_list / bsink_list with comp_dev_for_each_producer_safe() whose termination condition checks node->next == &comp->bsource_list. With the self-loop that check is always false. The walk runs inside irq_local_disable(), so the native_sim timer cannot preempt the thread and nsi_exec_for() never returns, making libFuzzer's max_total_time limit unreachable.

A second failure mode arises when ipc_buffer_free() calls pipeline_disconnect() on the corrupted buffer. list_item_del() updates comp->bsource_list.next to node->next, which due to the self-loop is the node itself — leaving the component's list head pointing into the freed buffer memory. When that memory is reused by a later allocation and overwritten, the next walk of bsource_list dereferences an invalid pointer, crashing with a null dereference or a corrupt-pointer access.

Add an early-exit guard in pipeline_connect() before buffer_attach(): if the buffer's relevant list node (source_list for COMP_TO_BUFFER, sink_list for BUFFER_TO_COMP) is not a self-loop singleton, the buffer is already attached and the connect is rejected with -EINVAL. list_is_empty() returns true only for a freshly created or correctly disconnected buffer, so no valid use case is rejected.

Verified with -s address on the full IPC3 corpus (~66K inputs, 75 s): zero crashes, zero hangs, ~2800 exec/s. The unfixed build stalled indefinitely on the same run.

Copilot AI review requested due to automatic review settings May 26, 2026 14:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the audio pipeline graph connection logic by preventing double-connects of the same buffer in a given direction, avoiding doubly-linked list corruption that can lead to hangs in ipc_comp_free() and potential use-after-free crashes during disconnect/free paths (notably observed via IPC3 persistent-mode fuzzing).

Changes:

  • Add an early-exit guard in pipeline_connect() to reject connections when the buffer’s relevant list node is already attached.
  • Emit an error and return -EINVAL on duplicate connect attempts.

* buffer-component pair (within one testcase, or via state carry-over
* between fuzzer testcases when IPC topology is not torn down).
*/
buf_list = dir == PPL_DIR_DOWNSTREAM ?
pipeline_connect() had no guard against being called twice for the same
buffer-component pair. Calling list_item_prepend() on a node that is
already in a doubly-linked list corrupts the list by creating a
self-loop where node->next points back to itself instead of to the list
head.

The corruption was discovered through IPC3 fuzzing in persistent mode.
Without per-testcase topology teardown, components and buffers created
by testcase N survive into testcase N+1. When N+1 sends a
TPLG_COMP_CONNECT for IDs that N already connected, ipc_comp_connect()
finds the surviving objects and calls pipeline_connect() a second time.
The same sequence can also be triggered within a single testcase by two
CONNECT messages for the same pair.

The self-loop causes ipc_comp_free() to hang indefinitely. The function
walks bsource_list / bsink_list with comp_dev_for_each_producer_safe()
whose termination condition checks node->next == &comp->bsource_list.
With the self-loop that check is always false. The walk runs inside
irq_local_disable(), so the native_sim timer cannot preempt the thread
and nsi_exec_for() never returns, making libFuzzer's max_total_time
limit unreachable.

A second failure mode arises when ipc_buffer_free() calls
pipeline_disconnect() on the corrupted buffer. list_item_del() updates
comp->bsource_list.next to node->next, which due to the self-loop is the
node itself — leaving the component's list head pointing into the freed
buffer memory. When that memory is reused by a later allocation and
overwritten, the next walk of bsource_list dereferences an invalid
pointer, crashing with a null dereference or a corrupt-pointer access.

Add an early-exit guard in pipeline_connect() before buffer_attach():
if the buffer's relevant list node (source_list for COMP_TO_BUFFER,
sink_list for BUFFER_TO_COMP) is not a self-loop singleton, the buffer
is already attached and the connect is rejected with -EINVAL.
list_is_empty() returns true only for a freshly created or correctly
disconnected buffer, so no valid use case is rejected.

Verified with -s address on the full IPC3 corpus (~66K inputs, 75 s):
zero crashes, zero hangs, ~2800 exec/s. The unfixed build stalled
indefinitely on the same run.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants