-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[BUG] Matter Thread Accessory Crash Due to Corner Case(TTL=0) #31031
Comments
According to this case, is it necessary to continue querying for the AAAA address if first query response only include SRV + TXT and the TTL is ZERO? What's the best practise here. Small change for OTBR to easy reproduce this issue. https://github.com/openthread/openthread/blob/main/src/core/net/dnssd_server.cpp#L618
After configuring this special otbr, let device perform operational discovery to reproduce. |
TTL of 0 in a DNS query is valid for GoodBye packets, when certain resource record data is about to become invalid: https://datatracker.ietf.org/doc/html/rfc6762#section-10.1. I am still trying to understand how https://github.com/project-chip/connectedhomeip/blob/master/src/lib/dnssd/Discovery_ImplPlatform.cpp#L325 - mTextEntrySize - gets an invalid value. |
|
@fuxiaoming-lumi , thx for the explanations, in the end I also discovered the failing flow. For sure we need to fix code that is handling a GoodBye Packet on the device side and maybe set Now, the question is on whether the GoodBye packet should reach the Matter device. My understanding is that the DNS client is sending a query for getting the IPv6 address of the OTA Provider. This query reaches the DNS server (running on the BR) which first consults the SRP cache and it can't find any entry. Then the DNS server is asking the mDNS proxy to forward the query on LAN. If the mDNS proxy receives the Goodbye Packet from the OTA Provider then probably it shouldn't forward it back to the Matter device but only delete its cache (if the mDNS proxy has a cache - this is optional):
|
I second @doru91 - it's not clear to me after glancing at RFCs if receiving RR with TTL of zero from the discovery proxy is valid. The original unicast DNS RFC allows RRs with TTL of zero:
Though given that in multicast DNS TTL of zero means the goodbye packet, I'm not sure in which cases the discovery proxy is allowed to return that as an answer to the standard DNS query. @fuxiaoming-lumi Which border router did you use to reproduce the original issue? |
@Damian-Nordic @doru91 Actually, we can reproduce this issue on SVE2 Raspberry Pi BR. And I agree with doru that we can set |
@fuxiaoming-lumi Thanks, yeah, let me post a PR with this change. I just wondered if any changes in OpenThread are needed as well. @abtink Do you maybe remember why OpenThread's DNS client discards TXT records with TTL 0 (see the code below), yet it returns SRV with TTL 0? I wonder how
|
@Damian-Nordic I am very grateful for this. I am not an expert in DNS-SD, so I am not familiar with the correct behavior as a DNS client for situations where TTL=0. I am also very curious whether it is necessary to inquire about the AAAA address, or when we receive TTL=0, we need to determine that this operational discovery has failed. https://github.com/project-chip/connectedhomeip/blob/master/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp#L2670 |
@fuxiaoming-lumi PTAL #31227. I tested it with your OTBR modification, but feel free to verify it on your own :). |
A TTL zero indicate TXT record is removed (no longer valid), so OT DNS client does sets |
Reproduction steps
When the accessory performs DNS-SD during operational discovery, it will receive the dns result.
The TTL maybe ZERO in a specific environment. And this will cause the device to crash.
We can 100% reproduce this issue base on lighting-app by Silabs efr32 and NXP K32W.
We get an illegal value(mTextEntrySize:662774400) causing the device to crash.
https://github.com/project-chip/connectedhomeip/blob/master/src/lib/dnssd/Discovery_ImplPlatform.cpp#L325
The key point is here: https://github.com/project-chip/connectedhomeip/blob/master/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp#L2622
When the TTL is 0, it is important to ensure that the variable is properly initialized before being passed to the upper layer. Failing to initialize the variable may result in unpredictable parameter values being propagated upwards.
We need to sort out this part of the logic to deal with the situation where TTL is 0 for Matter and define how this part of the behavior should be done correctly.
openthread: https://github.com/openthread/openthread/blob/main/src/core/net/dns_client.cpp#L346
Bug prevalence
100%
GitHub hash of the SDK that was being used
52e4a64
Platform
efr32, k32w
Platform Version(s)
N/A
Anything else?
Device Log:
Uploading C5DC8C3E-CDE4-42b1-BDB6-39536ADD7B57.txt…
DIS:
matter-efr32-lighting-example.zip
The text was updated successfully, but these errors were encountered: