schedule: dp: Double free fixes - #11217
Merged
Merged
Conversation
scheduler_dp_task_init() keeps its own copy of the component driver
inside task_memory and repoints mod->dev->drv at it:
task_memory->drv = *mod->dev->drv;
mod->dev->drv = &task_memory->drv;
Every error path ends at e_tmem, which releases task_memory, so
mod->dev->drv is left pointing into memory that has just been freed.
Nothing notices until the host tears the pipeline down and the module is
freed for real:
module_free(): ops = mod->dev->drv->adapter_ops
That dereference faults. The module heap is a vregion whose pages are
unmapped when it is released, so the access is rejected by the MMU
rather than quietly returning junk:
** FATAL EXCEPTION
** CPU 2 EXCCAUSE 28 (load prohibited)
** PC 0xa008297f
Backtrace: module_free <- module_adapter_free <- lib_manager_module_free
<- ipc4_delete_pipeline
Remember the original pointer and put it back before task_memory is
freed.
Verified on PTL by reverting the vpage reservation fix to bring back the
partition overlap that makes DP task creation fail: 18 consecutive
failures were reported to the host as errors with no heap corruption, no
exception and no panic, where previously the first one halted the core.
Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
requested review from
LaurentiuM1234,
dbaluta,
marcinszkudlinski and
pblaszko
as code owners
September 18, 2026 08:37
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The error-path cleanup safely restores the driver pointer before releasing task memory.
Pull request overview
Restores the original component-driver pointer when DP task initialization fails, preventing a dangling pointer during module teardown.
Changes:
- Saves the original driver pointer.
- Restores it before freeing task memory on error paths.
File summaries
| File | Description |
|---|---|
src/schedule/zephyr_dp_schedule_application.c |
Preserves and restores the component driver during failed task initialization. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
scheduler_dp_task_init() stored the new task in *task, which is the
caller's comp->task, immediately after initialising it - well before the
memory domain setup that can still fail. The task lives inside
task_memory, so every error path from that point on frees the object
that comp->task points at:
e_dom -> ... -> e_tmem: mod_free(mod, task_memory)
module_adapter_new_ext() then runs its own cleanup, which starts with
if (dev->task)
schedule_task_free(dev->task);
and so releases the same memory a second time. A failure that should
have been reported as a plain -EINVAL instead took the core down:
scheduler_dp_task_init: failed to add LLEXT to domain -22
module_adapter_new_ext: DP task creation failed with error -22.
sys_heap_free: heap corruption (double free?) at 0xa017fffc
** FATAL EXCEPTION ** CPU 2 EXCCAUSE 63 (zephyr exception)
>>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 2
A stale comp->task is harmful on its own as well, because
pipeline_comp_dp_task_init() returns early when it is set and would hand
out a dangling task on a later attempt.
Assign *task only after the last failure point, which is what the
non-userspace scheduler_dp_task_init() in zephyr_dp_schedule_thread.c
already does.
Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
changed the title
kv2019i
approved these changes
Sep 18, 2026
lyakh
approved these changes
Sep 21, 2026
lyakh
left a comment
lyakh
left a comment
Collaborator
There was a problem hiding this comment.
looks good, just need to make sure userspace DP tests pass too.
| k_thread_start(pdata->thread_id); | ||
|
|
||
| /* ptask points into task_memory, so only publish it once it cannot be freed */ | ||
| *task = ptask; |
Collaborator
There was a problem hiding this comment.
this looks good although it's a bit difficult to make sure this doesn't break anything by just looking at it. Have you run some DP tests with it to check?
Contributor
Author
There was a problem hiding this comment.
Yes I have. My usual "aplay -Dhw:0,2 -d 10 wav/sweep_32b2c44kHz.wav & arecord -Dhw:0,2 -f S16_LE -c 2 -r 32000 -d 10 -vvv -i recording.wav" works with this, but let's hope the real CI kicks in and tests this too. This was found when I debugged the llext relocatable stuff, that triggered many error paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
According to my tests the both fixes are needed. Howeve, the system still does not survive the error condition that I am hitting. But with these fixes the error happens later, and the simply looking at the function, they appear correct.