Switch instanceName used in FindCommissionableNode (backport #71877)#72374
Open
mergify[bot] wants to merge 1 commit into
Open
Switch instanceName used in FindCommissionableNode (backport #71877)#72374mergify[bot] wants to merge 1 commit into
mergify[bot] wants to merge 1 commit into
Conversation
* Switch instanceName used in FindCommissionableNode * Initialize mInstanceName (cherry picked from commit c71fb1f)
mergify
Bot
added
the
backport-v1.5-branch
|
PR #72374: Size comparison from 44aaaba to 62bf8f5 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
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.
Summary
Changes Made
This PR updates
UserDirectedCommissioningServer::HandleNewUDCso that the registeredInstanceNameResolveris given a pointer to theUDCClientState'sinstanceNameinstead of theIdentificationDeclaration'sinstanceName. TheUDCClientState's instance name is owned and persists long enough to prevent the flake from occurring.UDCClientState::GetInstanceNamereturns aconst char *, so we also updateFindCommissionableNodeto use const as well. This doesn't affect any callsites in the repo, but the change did need to be propagated across InstanceNameResolver subclasses.Background
JFADMIN 2.2 seems to require User-Directed Commissioning because each controller needs to initiate commissioning to get onto the corresponding administrator's fabric.
ASan reports a stack-use-after-return when doing a string compare on the following line inside
ActiveResolveAttempts::MarkPending:connectedhomeip/src/lib/dnssd/ActiveResolveAttempts.cpp
Line 140 in e6863fc
This code runs as part of
UserDirectedCommissioningServer::OnMessageReceived. It seems like an entry inActiveResolveAttempts'mRetryQueuehas a dangling pointer to the instanceName of an IdentificationDeclaration from an earlierOnMessageReceived. The variable is stack-allocated here:connectedhomeip/src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp
Line 63 in e6863fc
This is a rare flake because the UDC Server must receive a retransmission of the commissionee's identity declaration BEFORE mDNS completes and clears the RetryQueue. Retransmission occurs after 500 or more milliseconds, which is usually plenty of time for mDNS outside of CI.
Outside of ASan, this is probably a silent error because the string comparison just fails and a duplicate entry is added to the retry queue.
Here is a minimal test case showing the failure. If you add this to
TestActiveResolveAttempts.cpp, compile withis_asan=true chip_mdns="minimal", and run the test, it will crash.Disclaimer: I used AI to help troubleshoot and write the test, but everything written here is my own words from my own manual investigation of the failure/relevant code.
Related issues
#71404
^ Can't say with confidence this will be fixed, so I won't mark it to automatically close.
Testing
Ran TestUdcMessages and JFADMIN 2.2, both passed.
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines
This is an automatic backport of pull request #71877 done by [Mergify](https://mergify.com).