Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tmleman wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −7
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions src/audio/buffers/comp_buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions src/audio/pipeline/pipeline-graph.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions src/include/sof/audio/buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) ?
Expand Down
Loading