Skip to content

mempool can return success if no memory was available #14504

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

Closed
pdunaj opened this issue Mar 14, 2019 · 5 comments · Fixed by #14514
Closed

mempool can return success if no memory was available #14504

pdunaj opened this issue Mar 14, 2019 · 5 comments · Fixed by #14514
Assignees
Labels
area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@pdunaj
Copy link
Collaborator

pdunaj commented Mar 14, 2019

Describe the bug
We replicated a bug where assertion on data failed. Problem replicates randomly and rarely (I would say once per hour of operation). Quick investigation shown that application gets 0x4 from k_malloc. Quick look at the code shown that this is possible if _sys_mem_pool_block_alloc returns success but block.data is NULL.
Looking at _sys_mem_pool_block_alloc proved that this is possible as the loop that is allocating a block gives no guarantee of actually allocating (i.e. data can be NULL) but it is always returning zero.

As a bonus there is no lock taken when level_empty is called in the first loop so return from it can be invalid (it can be reported as nonempty when in fact it is). I am not 100% sure that giving a lock here would serve any purpose as in the end there is absolutely no guarantee what will be available when second loop of _sys_mem_pool_block_alloc is called.

To Reproduce
Steps to reproduce the behavior:
Allocate from various context close to no memory condition.

Expected behavior
`'k_malloc' should return NULL if no memory is available.

Impact
Showstopper

Screenshots or console output
N/A

Environment (please complete the following information):
ncs zephyr 745326266d9c32ba3aa67d5af1f727059d9a88e3 (from d9876be upstream)

Additional context
N/A

@pdunaj
Copy link
Collaborator Author

pdunaj commented Mar 14, 2019

I reviewed the code and it seems the problem can happen even if memory is available. See #14508

@pdunaj
Copy link
Collaborator Author

pdunaj commented Mar 14, 2019

FYI @andyross

@pdunaj
Copy link
Collaborator Author

pdunaj commented Mar 14, 2019

Fix pushed

@andyross
Copy link
Collaborator

There was an almost identical bug about a year ago, and I thought this was all fixed. May have been reintroduced? I'll take it.

@andyross andyross assigned andyross and pdunaj and unassigned andyross Mar 14, 2019
@andyross
Copy link
Collaborator

Sorry, reassign back. Didn't notice you had a patch up.

@rljordan-zz rljordan-zz added the priority: medium Medium impact/importance bug label Mar 15, 2019
pdunaj added a commit to pdunaj/zephyr that referenced this issue Mar 15, 2019
Return -ENOMEM if no block is available on any level.

Fixes: zephyrproject-rtos#14504

Signed-off-by: Pawel Dunaj <[email protected]>
pdunaj added a commit to pdunaj/zephyr that referenced this issue Mar 15, 2019
Do not perform early level usage check. This can lead to situation
where block is seen as available on level when it was taken from
the other context.

Fixes: zephyrproject-rtos#14504

Signed-off-by: Pawel Dunaj <[email protected]>
galak pushed a commit that referenced this issue Mar 19, 2019
Return -ENOMEM if no block is available on any level.

Fixes: #14504

Signed-off-by: Pawel Dunaj <[email protected]>
galak pushed a commit that referenced this issue Mar 19, 2019
Do not perform early level usage check. This can lead to situation
where block is seen as available on level when it was taken from
the other context.

Fixes: #14504

Signed-off-by: Pawel Dunaj <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants