{{ message }}
audio: up_down_mixer: fix use-after-free on init allocation failure#10976
Open
tmleman wants to merge 1 commit into
Open
audio: up_down_mixer: fix use-after-free on init allocation failure#10976tmleman wants to merge 1 commit into
tmleman wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a lifecycle/cleanup bug in the up_down_mixer module’s IPC4 adapter path by ensuring init() does not attempt to tear down its own comp_dev on early allocation failure, and by hardening the module .free callback against being invoked before private data is set.
Changes:
- Remove
comp_free(dev)fromup_down_mixer_init()whenmod_zalloc()fails; return-ENOMEMand let the module adapter framework unwind. - Add a NULL-guard in
up_down_mixer_free()to safely handle being called beforemod->priv.privateis assigned.
up_down_mixer_init() called comp_free(dev) when mod_zalloc() failed and
then returned -ENOMEM. That is a lifecycle violation: a module's init()
callback must never free its own comp_dev. On init failure the module
adapter framework already unwinds the component:
module_init() -> mod_free_all(mod)
module_adapter_new_ext() -> module_adapter_mem_free(mod) (err path)
The stray comp_free(dev) ran a second, full teardown (module_adapter_free
-> module .free callback + mod_free_all() + module_adapter_mem_free() +
freeing dev) that collided with the framework cleanup in two ways:
1. NULL dereference: comp_free() invoked the module .free callback
(up_down_mixer_free) before mod->priv.private had been assigned, so
module_get_private_data() returned NULL and the callback
dereferenced it. This is the SEGV observed first.
2. Use-after-free / double free: comp_free() had already freed (and,
under CONFIG_SYS_HEAP_SANITIZER_ASAN, poisoned) the module resources
and the comp_dev. Control then returned to module_init(), which
called mod_free_all() on that freed memory, after which the
module_adapter_new_ext() error path freed it once more via
module_adapter_mem_free().
Fix by removing the comp_free(dev) call and simply returning -ENOMEM,
which is exactly what every other module does on this path (aria, asrc,
copier, crossover, drc, ...): propagate the error and let the adapter
framework own the cleanup. As defensive hardening, also guard
up_down_mixer_free() against a NULL private pointer.
The bug was latent: with the static Zephyr memory pools these small
allocations never failed, so the error path was never taken. It became
reachable only after the native_sim host-heap change routed module
allocations onto the C library allocator, where sustained memory
pressure over many iterations eventually makes mod_zalloc() fail. Found
by the IPC4 libFuzzer target on native_sim under AddressSanitizer.
Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
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.

up_down_mixer_init() called comp_free(dev) when mod_zalloc() failed and then returned -ENOMEM. That is a lifecycle violation: a module's init() callback must never free its own comp_dev. On init failure the module adapter framework already unwinds the component:
module_init() -> mod_free_all(mod)
module_adapter_new_ext() -> module_adapter_mem_free(mod) (err path)
The stray comp_free(dev) ran a second, full teardown (module_adapter_free -> module .free callback + mod_free_all() + module_adapter_mem_free() + freeing dev) that collided with the framework cleanup in two ways:
NULL dereference: comp_free() invoked the module .free callback (up_down_mixer_free) before mod->priv.private had been assigned, so module_get_private_data() returned NULL and the callback dereferenced it. This is the SEGV observed first.
Use-after-free / double free: comp_free() had already freed (and, under CONFIG_SYS_HEAP_SANITIZER_ASAN, poisoned) the module resources and the comp_dev. Control then returned to module_init(), which called mod_free_all() on that freed memory, after which the module_adapter_new_ext() error path freed it once more via module_adapter_mem_free().
Fix by removing the comp_free(dev) call and simply returning -ENOMEM, which is exactly what every other module does on this path (aria, asrc, copier, crossover, drc, ...): propagate the error and let the adapter framework own the cleanup. As defensive hardening, also guard up_down_mixer_free() against a NULL private pointer.
The bug was latent: with the static Zephyr memory pools these small allocations never failed, so the error path was never taken. It became reachable only after the native_sim host-heap change routed module allocations onto the C library allocator, where sustained memory pressure over many iterations eventually makes mod_zalloc() fail. Found by the IPC4 libFuzzer target on native_sim under AddressSanitizer.