Repository navigation
Fix a few small memory leaks; Fix use-after-free in ubus module - #443
Merged
Merged
Conversation
Contributor
Author
|
@jow- The CI failures are unrelated to these changes. The failures are in the FFI module. |
jow-
force-pushed
the
fix/ubus-nl80211-leaks
branch
from
September 23, 2026 08:45
aecf1dd to
caa1258
Compare
Collaborator
Contributor
Author
|
I have been running ucode with #447 applied and have not seen the issue? do you want me to rebase and drop the commit? |
Collaborator
|
Given your feedback, I merged #447 now and would kindly ask you to rebase your fixes on top of current head, so that I can merge them. |
uc_nl_request_common() allocates the request message before resolving the generic netlink family id. When that lookup fails, the error return skips nlmsg_free(), leaking the nl_msg and its payload buffer on every call. The family cache is never refreshed, so a process that asks before the driver is loaded keeps leaking for the rest of its life. Signed-off-by: Matthew Cather <mattbob4@gmail.com>
uc_ubus_call_cb() converts every data reply into a ucode value, but keeps only the first one unless the caller asked for multiple returns. Further replies from a multi reply method were converted and then dropped without a release, leaking one value per extra reply on every such call. Signed-off-by: Matthew Cather <mattbob4@gmail.com>
uc_ubus_call_cb() converts each reply into a ucode value and stores it in res->res as the data arrives, before the request status is known. ubus_complete_request() keeps polling after that and can still finish with UBUS_STATUS_TIMEOUT, or with UBUS_STATUS_CONNECTION_FAILED when the peer goes away mid reply. A method may also send a reply and then return an error status. Both callers, uc_ubus_call() and uc_ubus_chan_request(), take err_return() on a non-OK status, which is set_error() followed by return NULL, so the value in res->res is never released. In multiple return mode the whole accumulated reply array is lost. Release it in uc_ubus_call_common() so both callers are covered, and clear the field so a caller cannot hand back a stale pointer. The early errval_return() paths all sit before ubus_invoke_async_fd(), so no data callback can have run and res->res is still NULL there. Signed-off-by: Matthew Cather <mattbob4@gmail.com>
MattCatz
force-pushed
the
fix/ubus-nl80211-leaks
branch
from
October 28, 2026 23:08
caa1258 to
dc5fd36
Compare
Contributor
Author
Collaborator
|
Merged, thanks! |
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.
I have been experimenting with some patches that enable the Hardware-assisted AddressSanitizer on aarch64 OpenWRT targets. Part of that included instrumenting ucode and libucode with the sanitizer for fun and profit. Four issues surfaced and they seem to be valid. Three of them are small memory leaks. One, 0529943, is a use after-free-fix.
These are my purposed fixes. I can include test cases if needed.