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

[bt] fix if allocation fails (IDFGH-8518) #9972

Closed
wants to merge 1 commit into from

Conversation

tgotic
Copy link
Contributor

@tgotic tgotic commented Oct 13, 2022

If osi_malloc fails for work_queues or osi_work_queue_create fails, osi_work_queue_delete in _err may release unallocated memory.

If osi_malloc fails for work_queues or osi_work_queue_create fails, osi_work_queue_delete in _err may release unallocated memory.
@espressif-bot espressif-bot added the Status: Opened Issue is new label Oct 13, 2022
@github-actions github-actions bot changed the title [bt] fix if allocation fails [bt] fix if allocation fails (IDFGH-8518) Oct 13, 2022
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

Copy link
Contributor

@AxelLin AxelLin left a comment

Choose a reason for hiding this comment

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

The osi_calloc changes are not related to the issue.

And if thread->work_queues is NULL, it will hit NULL pointer dereference in error path.

@tgotic
Copy link
Contributor Author

tgotic commented Oct 17, 2022

case 1:
with osi_malloc:
osi_thread_t *thread = osi_malloc()
success, thread->work_queue_num is random number, thread->thread_handle is random address
thread->work_queues = osi_malloc()
fails, goto _err:
if (thread->thread_handle) {
delete random task

for (int i = 0; i < thread->work_queue_num; i++) { if (thread->work_queues[i]) - NULL pointer dereference

case 2:
osi_thread_t *thread = osi_malloc()
success, thread->work_queue_num is random number
thread->work_queues = osi_malloc()
success, thread->work_queues[] are random, not NULL
thread->work_queue_num = work_queue_num

for (int i = 0; i < thread->work_queue_num; i++) { thread->work_queues[i] = osi_work_queue_create(queue_len);
fails for not last, goto _err;
_err: for (int i = 0; i < thread->work_queue_num; i++) { if (thread->work_queues[i]) {
free random unallocated memory

with osi_calloc:
case 1:
osi_thread_t *thread = osi_calloc()
success, thread->work_queue_num is 0, thread->thread_handle is NULL
thread->work_queues = osi_calloc()
fails, goto _err:
if (thread->thread_handle) {
thread->thread_handle is NULL, won't delete task

for (int i = 0; i < thread->work_queue_num; i++) {
won't go here, thread->work_queue_num is 0

case 2:
osi_thread_t *thread = osi_calloc()
success, thread->work_queue_num is 0, thread->thread_handle is NULL
thread->work_queues = osi_calloc()
success, thread->work_queues[] are NULL
thread->work_queue_num = work_queue_num

for (int i = 0; i < thread->work_queue_num; i++) { thread->work_queues[i] = osi_work_queue_create(queue_len);
fails for not last, goto _err;
_err:
for (int i = 0; i < thread->work_queue_num; i++) { if (thread->work_queues[i]) {
won't go here, thread->work_queues[i] are NULL

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Resolution: NA Issue resolution is unavailable labels Oct 25, 2022
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Alvin1Zhang
Copy link
Collaborator

Changes merged with 2d2d0ba, thanks for contribution again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants