Skip to content

Conversation

@lgirdwood
Copy link
Member

Fix NULL pointer dereference crash in copier module by using the correct IPC4 macro to extract queue IDs from buffer IDs.

The issue occurred in do_conversion_copy() and copier_module_copy() when accessing cd->converter[i] where i was extracted using IPC4_SINK_QUEUE_ID().

This was incorrect because buffer IDs are constructed as: IPC4_COMP_ID(src_queue, dst_queue)

From the buffer's perspective, the copier's sink is actually the source, so IPC4_SRC_QUEUE_ID() should be used to get the correct copier sink index.

Using IPC4_SINK_QUEUE_ID() extracted the dst_queue (upper 16 bits) instead of src_queue (lower 16 bits), leading to wrong array indices and NULL pointer crashes when the converter array wasn't initialized for those indices.

This resolves crashes in RTC AEC topologies where internal module copiers have buffer IDs that map to non-zero queue IDs.

Fix NULL pointer dereference crash in copier module by using the correct
IPC4 macro to extract queue IDs from buffer IDs.

The issue occurred in do_conversion_copy() and copier_module_copy() when
accessing cd->converter[i] where i was extracted using
IPC4_SINK_QUEUE_ID().

This was incorrect because buffer IDs are constructed as:
IPC4_COMP_ID(src_queue, dst_queue)

From the buffer's perspective, the copier's sink is actually the source,
so IPC4_SRC_QUEUE_ID() should be used to get the correct copier sink
index.

Using IPC4_SINK_QUEUE_ID() extracted the dst_queue (upper 16 bits)
instead of src_queue (lower 16 bits), leading to wrong array indices and
NULL pointer crashes when the converter array wasn't initialized for
those indices.

This resolves crashes in RTC AEC topologies where internal module
copiers have buffer IDs that map to non-zero queue IDs.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
if (!audio_buffer_hw_params_configured(&src->audio_buffer) ||
!audio_buffer_hw_params_configured(&sink->audio_buffer))
return 0;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally this would be with "git cherry-pick -x " to have a ref to the mainline commit.

Copy link
Collaborator

@softwarecki softwarecki left a comment

Choose a reason for hiding this comment

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

I believe we need to stop treating the buffer as a component and stop reasoning about data flow direction from its perspective. Using interchangeable meanings depending on the point of view leads to these kinds of mistakes - especially when a component accesses the buffer structure and extracts something from it. Once you're inside the buffer structure, the naming is reversed.
I like the approach proposed in the sink/source API, where components write to a sink and read from a source. Ultimately, with the transition to pipeline 2.0 and the adoption of sink/source, I'd prefer to see separate functions for retrieving IDs.

@lgirdwood lgirdwood merged commit b6ecb8f into thesofproject:stable-v2.14 Oct 13, 2025
32 of 44 checks passed
@lgirdwood lgirdwood deleted the stable-v2.14 branch October 13, 2025 10:47
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.

5 participants