Skip to content
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.

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

zephyr: alloc: virtual_heap_free: Panic on deallocations errors #9779

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

softwarecki
Copy link
Collaborator

@softwarecki softwarecki commented Jan 16, 2025

Add k_panic() function call in error handling code to help detect potential memory release errors.

The vmh_free function returns an error if:

  1. heap belongs to another core,
  2. given pointer to be freed is invalid (doesn't belong to the allocator),
  3. there is an error in the code determining the size of the block to be freed
  4. memory unmapping fails.

Log entry is easy to miss, stopping the firmware at this point will draw attention to a critical problem related to memory allocation. Otherwise, it will have a slowly progressing memory leak.

If we decide this is a desirable change, we can merge it after #9776 and #9777 have been merged.

Add k_panic() function call in error handling code to help detect potential
memory release errors.

The vmh_free function returns an error if:
1. heap belongs to another core,
2. given pointer to be freed is invalid (doesn't belong to the allocator),
3. there is an error in the code determining the size of the block to be
   freed
4. memory unmapping fails.

Log entry is easy to miss, stopping the firmware at this point will draw
attention to a critical problem related to memory allocation. Otherwise,
it will have a slowly progressing memory leak.

Signed-off-by: Adrian Warecki <[email protected]>
tr_err(&zephyr_tr, "Unable to free %p! %d", ptr, ret);
k_panic();
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want to panic ? e.g. would a user space module be able to panic with a bad free() ? (and stop all other pipelines)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really want to panic ? e.g. would a user space module be able to panic with a bad free() ? (and stop all other pipelines)

+1 - maybe we should add some notification to the host about similar important but not fatal errors

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