fix(WiFi/ESP32): check cb_event for NULL before calling on station connect/disconnect#57
Open
94xhn wants to merge 1 commit into
Open
fix(WiFi/ESP32): check cb_event for NULL before calling on station connect/disconnect#5794xhn wants to merge 1 commit into
94xhn wants to merge 1 commit into
Conversation
…nnect/disconnect ARM_WIFI_Initialize() accepts a NULL cb_event when the caller doesn't need event callbacks, and this is documented as a valid usage. But AT_Notify() calls pCtrl->cb_event() unconditionally for AT_NOTIFY_STATION_CONNECTED/AT_NOTIFY_STATION_DISCONNECTED (which raise ARM_WIFI_EVENT_AP_CONNECT/ARM_WIFI_EVENT_AP_DISCONNECT), with no NULL check - a null pointer call if a station connects to or disconnects from the local AP while no callback was registered. Fixes ARM-software#13. Signed-off-by: 94xhn <87560781+94xhn@users.noreply.github.com>
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
1 participant
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.
Fixes #13.
ARM_WIFI_Initialize()accepts acb_eventcallback that, per the driver's documented behavior, may beNULLif the caller doesn't need event callbacks.pCtrl->cb_eventis also explicitly set toNULLonUninitialize().However
AT_Notify()callspCtrl->cb_event(...)unconditionally forAT_NOTIFY_STATION_CONNECTEDandAT_NOTIFY_STATION_DISCONNECTED(which raiseARM_WIFI_EVENT_AP_CONNECT/ARM_WIFI_EVENT_AP_DISCONNECTrespectively) - the only two call sites ofcb_eventin this function - with no NULL check. As the issue describes, this triggers whenever a station connects to or disconnects from the local AP (e.g. a remote host dropping its connection) while no event callback was registered, resulting in a null pointer call.Fix: guard both call sites with
if (pCtrl->cb_event != NULL), exactly as suggested in the issue and confirmed by @VladimirUmek's comment ("this check shall be added").Test plan
No physical ESP32 hardware/AT-command host available to exercise this path end-to-end. Verified by code inspection: confirmed these are the only two
cb_eventinvocations insideAT_Notify(), confirmedcb_eventcan legitimately beNULLperARM_WIFI_Initialize()'s documented parameter contract and itsUninitialize()reset, and confirmed no other AT_NOTIFY_* branch in the same function callscb_event(so no other call sites need the same guard).Disclosure
Generative AI (Claude) was used to help investigate this and implement the fix. All changes were reviewed by me before submission.