chain dma: fix oops at stream close#11030
Conversation
… during task free When a task is freed by zephyr_ll_task_free(), zephyr_ll_run() can still hold a reference to the task's pdata structure across the LL lock boundary. Re-fetching task->priv_data under zephyr_ll_lock() prevents evaluating pdata->freeing on a stale/freed pointer. Additionally, add NULL checks for pdata in zephyr_ll_task_done() and task/pdata in zephyr_ll_task_free(). Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…p_thread_ipc Move task_dp_pdata assignment after checking pmod and validate pmod->dev, pmod->dev->task, and pmod->dev->task->priv_data pointers before dereferencing to prevent NULL pointer crashes. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Validate cd, cd->dma_buffer, cd->chan_link, and cd->chan_host at the start of chain_task_run() to handle stream teardown cleanly when buffers or channels are freed before task completion. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
| task->state = SOF_TASK_STATE_FREE; | ||
|
|
||
| if (pdata->freeing) | ||
| if (pdata && pdata->freeing) |
There was a problem hiding this comment.
Rather than checking here, could this be fixed at source to ensure the data is valid through the life cycle of zephyr_ll_task_free() ?
lyakh
left a comment
lyakh
left a comment
There was a problem hiding this comment.
sorry, I don't understand the reasoning why specifically these 3 locations were picked to check pointers.
| return -EINVAL; | ||
| } | ||
|
|
||
| struct task_dp_pdata *pdata = pmod->dev->task->priv_data; |
There was a problem hiding this comment.
what is the path that can lead to any of those pointers being NULL? I don't think evaluating all pointers in each function is a good goal. Besides, these pointers aren't protected, so nothing prevents this thread from being preempted between the check and the dereference and setting any of those pointers to NULL
| int ret; | ||
|
|
||
| if (!cd || !cd->dma_buffer || !cd->chan_link || !cd->chan_host) | ||
| return SOF_TASK_STATE_COMPLETED; |
There was a problem hiding this comment.
ditto, what is the scenario that can lead to these being NULL?
You need to enable breadcrumbs to help see the failure paths, the agent is just covering here based on this data, closing now sine we can apply the revert. |
3 participants
Some of this duplicates @lyakh PR, but agent provided some additional patches to prevent the NULL deref. Test in CI now and review comments will be folded before merge. Likely drop 1/3 in favour of @lyakh update.