Skip to content

Conversation

@bjarki-andreasen
Copy link
Contributor

@bjarki-andreasen bjarki-andreasen commented Sep 5, 2023

The net_mgmt subsystem offers a function which waits (blocks) until a specified net event occurs. An event callback is pushed to the stack, then added to the net_mgmt_event_callback list. If the event occurs, the net_mgmt thread calls the callback and deletes the callback from the list. However, if the event does not occur within the timeout specified when invoking mgmt_event_wait_call() the function will return, corrupting the callback structure when the stack is reused.

This PR fixes the issue by deleting the callback before exiting in case the event does not occur.

@rerickson1 I found it! #56692 (comment)

rerickson1
rerickson1 previously approved these changes Sep 5, 2023
Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

LGTM, minor nit about the commit subject, please change it to
net: mgmt: Fix memory corruption in wait_on_iface

@bjarki-andreasen bjarki-andreasen force-pushed the bugfix_net_mgmt_wait_on_iface_memory_corruption branch from 81d63ed to b07a835 Compare September 6, 2023 06:12
@bjarki-andreasen
Copy link
Contributor Author

bjarki-andreasen commented Sep 6, 2023

Updated commit subject to net: mgmt: Fix memory corruption in wait_on_iface

the build test failing is not due to changes in this PR :)

@jukkar
Copy link
Member

jukkar commented Sep 6, 2023

the build test failing is not due to changes in this PR :)

The issue was fixed by #62317, could you rebase and re-submit.

tbursztyka
tbursztyka previously approved these changes Sep 6, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

could you rewrite the above as such:

if (ret < 0) {
     if (ret == -EAGAIN) {
         ret = -ETIMEDOUT;
    }

    net_mgmt_del_event_callback(&sync);
    return ret;
}

it's shorter/clearer I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewritten as you have suggested :)

See last force-push

@bjarki-andreasen bjarki-andreasen force-pushed the bugfix_net_mgmt_wait_on_iface_memory_corruption branch from b07a835 to f785192 Compare September 6, 2023 14:32
The net_mgmt subsystem offers a function which waits (blocks)
until a specified net event occurs. An event callback is
pushed to the stack, then added to the net_mgmt_event_callback
list. If the event occurs, the net_mgmt thread calls the
callback and deletes the callback from the list. However, if
the event does not occur within the timeout specified when
invoking mgmt_event_wait_call() the function will return,
corrupting the callback structure the stack is reused.

This PR fixes the issue by deleting the callback before exiting
in case the event does not occur.

Signed-off-by: Bjarki Arge Andreasen <bjarkix123@gmail.com>
@bjarki-andreasen bjarki-andreasen force-pushed the bugfix_net_mgmt_wait_on_iface_memory_corruption branch from f785192 to 4c65cbf Compare September 6, 2023 14:35
@bjarki-andreasen
Copy link
Contributor Author

@jukkar Rebased on main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants