Skip to content

1.8: consomme: fix deadlock and UAF in Windows DNS resolver (#4032) - #4122

Merged
Steven Malis (smalis-msft) merged 1 commit into
microsoft:release/1.8.2607from
smalis-msft:backport-1.8-pr4032
Aug 4, 2026
Merged

1.8: consomme: fix deadlock and UAF in Windows DNS resolver (#4032)#4122
Steven Malis (smalis-msft) merged 1 commit into
microsoft:release/1.8.2607from
smalis-msft:backport-1.8-pr4032

Conversation

@smalis-msft

Copy link
Copy Markdown
Contributor

Backport of #4032 to release/1.8.2607.

The cherry-pick of ed36005 applied cleanly onto release/1.8.2607 with no conflicts and no manual edits.

Original PR: #4032


This backport PR was created by an AI agent (GitHub Copilot) on behalf of Steven Malis (@smalis-msft).

We had a DNS vmm test flake by timing out. The lock on pending_requests is being held inside cancel_all while a
completion called the callback, which tries to take the same mutex, resulting in a deadlock. Fix this by not holding the lock over the calls to DnsCancelQueryRaw, but also narrow the lock windows everywhere else too. Then wrap the handle in an Arc to ensure that it stays alive across all possible asynchronous flows, fixing a potential UAF.

(cherry picked from commit ed36005)
Copilot AI review requested due to automatic review settings July 31, 2026 17:18
@smalis-msft
Steven Malis (smalis-msft) requested a review from a team as a code owner July 31, 2026 17:18
@smalis-msft Steven Malis (smalis-msft) added the release_1.8.2607 Targets the release/1.8.2607 branch. label Jul 31, 2026
@github-actions github-actions Bot added the unsafe Related to unsafe code label Jul 31, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Backports the Windows DnsQueryRaw-based resolver fix from #4032 to the release/1.8.2607 branch, addressing a deadlock during cancellation and a potential use-after-free by ensuring the DNS cancel handle remains alive across async callback/cancel flows.

Changes:

  • Avoid holding the pending_requests mutex while calling DnsCancelQueryRaw (prevents self-deadlock when callbacks re-enter).
  • Store cancel handles as Arc<RawCancelHandle> to keep the underlying DNS_QUERY_RAW_CANCEL allocation alive across async paths.
  • Narrow pending_requests lock windows in query() and cancel_all().
Suppressed comments (1)

vm/devices/net/net_consomme/consomme/src/dns_resolver/windows/mod.rs:282

  • Slab::remove will panic if the key is already absent. Because this is invoked from an OS callback (and entries can be removed via other paths), prefer try_remove here to avoid turning unexpected callback ordering into a process panic.
    let _cancel_handle = context.pending_requests.lock().remove(context.slab_key);

// Remove placeholder since callback won't fire on error
if result != DNS_REQUEST_PENDING {
// Remove the cancel handle since the callback won't fire on error.
self.pending_requests.lock().remove(slab_key);
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

@smalis-msft
Steven Malis (smalis-msft) merged commit 0850d01 into microsoft:release/1.8.2607 Aug 4, 2026
124 of 126 checks passed
@smalis-msft
Steven Malis (smalis-msft) deleted the backport-1.8-pr4032 branch August 4, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_1.8.2607 Targets the release/1.8.2607 branch. unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants