[dns-client] fix double-free of mSavedResponse on duplicate response

[jwhui]

Fix a double-free of mSavedResponse in Dns::Client when processing duplicate DNS responses matching an active query.

When an SRV/TXT query needs to resolve a host address (AAAA), the DNS client allocates a chained newQuery to handle it. If duplicate responses are processed before the query chain is finalized, they trigger multiple AAAA resolution allocations for the same parent query. Because the new query inherits mSavedResponse from the parent query's QueryInfo, multiple chained queries end up aliasing/sharing the same cloned mSavedResponse message. During finalization, FreeQuery walks the chain and frees mSavedResponse for each query, leading to a double-free of the shared Message and free-list/heap corruption.

This commit resolves the issue by:

1. Rejecting duplicate responses early in ResolveHostAddressIfNeeded if a response has already been received and saved for the query (info.mSavedResponse != nullptr).
2. Initializing the mSavedResponse field of the QueryInfo struct to nullptr before allocating the host resolution query (newQuery) to prevent it from inheriting a potentially non-null saved response from its parent.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical memory corruption issue in the DNS client where duplicate responses triggered multiple chained queries to share the same saved response object. By ensuring that duplicate responses are rejected early and that new queries are properly initialized with a null saved response pointer, the fix prevents heap corruption during the query finalization process.

Highlights

• Duplicate Response Handling: Added a verification check to reject duplicate DNS responses if a response has already been saved for the current query.
• Memory Management: Initialized the mSavedResponse field to nullptr when allocating new chained queries to prevent them from inheriting and sharing the parent's saved response, which previously caused double-free errors.

New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

Merging #13060 into main

name	branch	text	data	bss	total
ot-cli-ftd	aae952a	483376	860	66564	550800
	c686100	483376	860	66564	550800
	+/-	+0	+0	+0	+0
ot-ncp-ftd	aae952a	449276	764	61800	511840
	c686100	449292	764	61800	511856
	+/-	+16	+0	+0	+16
ot-cli-mtd	aae952a	375344	764	50972	427080
	c686100	375360	764	50972	427096
	+/-	+16	+0	+0	+16
ot-ncp-mtd	aae952a	356508	764	46240	403512
	c686100	356524	764	46240	403528
	+/-	+16	+0	+0	+16
ot-cli-ftd-br	aae952a	599768	864	136764	737396
	c686100	599784	864	136764	737412
	+/-	+16	+0	+0	+16
ot-rcp	aae952a	63408	568	20812	84788
	c686100	63408	568	20812	84788
	+/-	+0	+0	+0	+0

Library files

name	branch	text	data	bss	total
libopenthread-ftd.a	aae952a	251612	95	40343	292050
	c686100	251624	95	40343	292062
	+/-	+12	+0	+0	+12
libopenthread-cli-ftd.a	aae952a	61853	0	8091	69944
	c686100	61853	0	8091	69944
	+/-	+0	+0	+0	+0
libopenthread-ncp-ftd.a	aae952a	33511	0	5948	39459
	c686100	33511	0	5948	39459
	+/-	+0	+0	+0	+0
libopenthread-mtd.a	aae952a	170112	0	24783	194895
	c686100	170124	0	24783	194907
	+/-	+12	+0	+0	+12
libopenthread-cli-mtd.a	aae952a	41585	0	8059	49644
	c686100	41585	0	8059	49644
	+/-	+0	+0	+0	+0
libopenthread-ncp-mtd.a	aae952a	25807	0	5948	31755
	c686100	25807	0	5948	31755
	+/-	+0	+0	+0	+0
libopenthread-ftd-br.a	aae952a	368940	99	110503	479542
	c686100	368952	99	110503	479554
	+/-	+12	+0	+0	+12
libopenthread-cli-ftd-br.a	aae952a	81052	0	8131	89183
	c686100	81052	0	8131	89183
	+/-	+0	+0	+0	+0
libopenthread-rcp.a	aae952a	9946	0	5060	15006
	c686100	9946	0	5060	15006
	+/-	+0	+0	+0	+0
libopenthread-radio.a	aae952a	19973	0	246	20219
	c686100	19973	0	246	20219
	+/-	+0	+0	+0	+0

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.23%. Comparing base (aae952a) to head (21c7ab7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13060      +/-   ##
==========================================
+ Coverage   74.15%   74.23%   +0.08%     
==========================================
  Files         696      695       -1     
  Lines       95114    93401    -1713     
==========================================
- Hits        70528    69334    -1194     
+ Misses      24586    24067     -519     

Files with missing lines	Coverage Δ
src/core/net/dns_client.cpp	78.66% <100.00%> (-3.04%)	⬇️

... and 95 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jwhui]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the DNS client to prevent pointer aliasing by ensuring mSavedResponse is nullified during query initialization and verified during response parsing. The reviewer recommends checking other functions that copy QueryInfo for similar issues and suggests centralizing the initialization logic within AllocateQuery to improve long-term robustness.

I am having trouble creating individual review comments. Click here to see my feedback.

src/core/net/dns_client.cpp (1876)

This fix correctly prevents the new sub-query from inheriting the parent's mSavedResponse pointer. It is worth verifying if other functions that allocate new queries by copying QueryInfo (such as ReplaceWithSeparateSrvTxtQueries or ReplaceWithIp4Query) also need to explicitly clear mSavedResponse and mNextQuery to avoid similar pointer aliasing issues. Centralizing this clearing logic within AllocateQuery (by having it always clear these fields in the written QueryInfo) might be a more robust long-term solution.

References

1. Ensure that structures are properly initialized or cleared to maintain invariants and avoid pointer aliasing issues, similar to the requirement for Metadata structures to be zero-initialized in Clear().

[abtink]

👍