Skip to content

Commit b665fcb

Browse files
committed
pipeline: reject double-connect of already-attached buffer
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 <tomasz.m.leman@intel.com>
1 parent 4241c75 commit b665fcb

3 files changed

Lines changed: 41 additions & 7 deletions

File tree

src/audio/buffers/comp_buffer.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -622,13 +622,6 @@ void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes)
622622
#endif
623623
}
624624

625-
static inline struct list_item *buffer_comp_list(struct comp_buffer *buffer,
626-
int dir)
627-
{
628-
return dir == PPL_DIR_DOWNSTREAM ?
629-
&buffer->source_list : &buffer->sink_list;
630-
}
631-
632625
/*
633626
* Locking: must be called with interrupts disabled (or sys_mutex held for
634627
* userspace LL builds)! Serialized IPCs protect us

src/audio/pipeline/pipeline-graph.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer,
198198
int dir)
199199
{
200200
struct list_item *comp_list;
201+
struct list_item *buf_list;
201202
PPL_LOCK_DECLARE;
202203

203204
if (dir == PPL_CONN_DIR_COMP_TO_BUFFER)
@@ -207,6 +208,35 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer,
207208

208209
PPL_LOCK();
209210

211+
/*
212+
* Guard against double-connecting the same buffer. Calling
213+
* list_item_prepend() on a node that is already in a list creates a
214+
* self-loop (node->next == node) that permanently corrupts the list.
215+
* Consequences:
216+
* - ipc_comp_free() enters an unbounded loop inside irq_local_disable,
217+
* stalling the simulation indefinitely.
218+
* - pipeline_disconnect() / list_item_del() fails to unlink the buffer
219+
* from the component, leaving a dangling pointer that causes
220+
* use-after-free when the buffer is later freed.
221+
* This can be triggered by a second IPC CONNECT message for the same
222+
* buffer-component pair (within one testcase, or via state carry-over
223+
* between fuzzer testcases when IPC topology is not torn down).
224+
*
225+
* list_is_empty() on an embedded list node (not a list head) returns
226+
* true when the node is unlinked (node->next == node, set by
227+
* list_init() at creation or after list_item_del()). Once linked into
228+
* a component's buffer list, node->next points elsewhere and
229+
* list_is_empty() returns false — meaning the buffer is already
230+
* connected.
231+
*/
232+
buf_list = buffer_comp_list(buffer, dir);
233+
if (!list_is_empty(buf_list)) {
234+
comp_err(comp, "buffer %d already connected dir %d",
235+
buf_get_id(buffer), dir);
236+
PPL_UNLOCK();
237+
return -EINVAL;
238+
}
239+
210240
comp_list = comp_buffer_list(comp, dir);
211241
buffer_attach(buffer, comp_list, dir);
212242
buffer_set_comp(buffer, comp, dir);

src/include/sof/audio/buffer.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,17 @@ void buffer_attach(struct comp_buffer *buffer, struct list_item *head, int dir);
300300
*/
301301
void buffer_detach(struct comp_buffer *buffer, struct list_item *head, int dir);
302302

303+
/**
304+
* Get the buffer's list node used to link it into a component's buffer list.
305+
* For PPL_DIR_DOWNSTREAM the buffer is a source to the component (source_list),
306+
* for PPL_DIR_UPSTREAM the buffer is a sink (sink_list).
307+
*/
308+
static inline struct list_item *buffer_comp_list(struct comp_buffer *buffer, int dir)
309+
{
310+
return dir == PPL_DIR_DOWNSTREAM ?
311+
&buffer->source_list : &buffer->sink_list;
312+
}
313+
303314
static inline struct comp_dev *buffer_get_comp(struct comp_buffer *buffer, int dir)
304315
{
305316
struct comp_dev *comp = (dir == PPL_DIR_DOWNSTREAM) ?

0 commit comments

Comments
 (0)