From b665fcbfd97fc5a5cdfc5b5cdd94bd572755bfa0 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Tue, 26 May 2026 17:39:39 +0200 Subject: [PATCH] pipeline: reject double-connect of already-attached buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Move buffer_comp_list() from a file-static helper in comp_buffer.c to a public static inline in buffer.h alongside similar accessors like buffer_get_comp(). This enables reuse in pipeline_connect() where the same direction-to-list-node mapping is needed for the guard check. Add an early-exit guard in pipeline_connect() before buffer_attach(): use buffer_comp_list() to obtain the relevant list node, then check list_is_empty(). On a list node (not a list head), list_is_empty() returns true only when node->next == node, meaning the node is unlinked. If it returns false the buffer is already connected and the call is rejected with -EINVAL. Verified with -s address on the full IPC3 corpus (~95K runs, 41 s): zero crashes, zero hangs, ~2300 exec/s. The unfixed build stalled indefinitely on the same run. Signed-off-by: Tomasz Leman --- src/audio/buffers/comp_buffer.c | 7 ------- src/audio/pipeline/pipeline-graph.c | 30 +++++++++++++++++++++++++++++ src/include/sof/audio/buffer.h | 11 +++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/audio/buffers/comp_buffer.c b/src/audio/buffers/comp_buffer.c index cc38a7dfead7..0902af84d986 100644 --- a/src/audio/buffers/comp_buffer.c +++ b/src/audio/buffers/comp_buffer.c @@ -622,13 +622,6 @@ void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes) #endif } -static inline struct list_item *buffer_comp_list(struct comp_buffer *buffer, - int dir) -{ - return dir == PPL_DIR_DOWNSTREAM ? - &buffer->source_list : &buffer->sink_list; -} - /* * Locking: must be called with interrupts disabled (or sys_mutex held for * userspace LL builds)! Serialized IPCs protect us diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 47d5d0127fd0..adac7d3f477d 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -198,6 +198,7 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, int dir) { struct list_item *comp_list; + struct list_item *buf_list; PPL_LOCK_DECLARE; if (dir == PPL_CONN_DIR_COMP_TO_BUFFER) @@ -207,6 +208,35 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, PPL_LOCK(); + /* + * Guard against double-connecting the same buffer. Calling + * list_item_prepend() on a node that is already in a list creates a + * self-loop (node->next == node) that permanently corrupts the list. + * Consequences: + * - ipc_comp_free() enters an unbounded loop inside irq_local_disable, + * stalling the simulation indefinitely. + * - pipeline_disconnect() / list_item_del() fails to unlink the buffer + * from the component, leaving a dangling pointer that causes + * use-after-free when the buffer is later freed. + * This can be triggered by a second IPC CONNECT message for the same + * buffer-component pair (within one testcase, or via state carry-over + * between fuzzer testcases when IPC topology is not torn down). + * + * list_is_empty() on an embedded list node (not a list head) returns + * true when the node is unlinked (node->next == node, set by + * list_init() at creation or after list_item_del()). Once linked into + * a component's buffer list, node->next points elsewhere and + * list_is_empty() returns false — meaning the buffer is already + * connected. + */ + buf_list = buffer_comp_list(buffer, dir); + if (!list_is_empty(buf_list)) { + comp_err(comp, "buffer %d already connected dir %d", + buf_get_id(buffer), dir); + PPL_UNLOCK(); + return -EINVAL; + } + comp_list = comp_buffer_list(comp, dir); buffer_attach(buffer, comp_list, dir); buffer_set_comp(buffer, comp, dir); diff --git a/src/include/sof/audio/buffer.h b/src/include/sof/audio/buffer.h index 6e6b8a9caef8..6f591dd528b7 100644 --- a/src/include/sof/audio/buffer.h +++ b/src/include/sof/audio/buffer.h @@ -300,6 +300,17 @@ void buffer_attach(struct comp_buffer *buffer, struct list_item *head, int dir); */ void buffer_detach(struct comp_buffer *buffer, struct list_item *head, int dir); +/** + * Get the buffer's list node used to link it into a component's buffer list. + * For PPL_DIR_DOWNSTREAM the buffer is a source to the component (source_list), + * for PPL_DIR_UPSTREAM the buffer is a sink (sink_list). + */ +static inline struct list_item *buffer_comp_list(struct comp_buffer *buffer, int dir) +{ + return dir == PPL_DIR_DOWNSTREAM ? + &buffer->source_list : &buffer->sink_list; +} + static inline struct comp_dev *buffer_get_comp(struct comp_buffer *buffer, int dir) { struct comp_dev *comp = (dir == PPL_DIR_DOWNSTREAM) ?