Skip to content

asyn-thread: stop using GetAddrInfoExW on Windows#14794

Closed
jay wants to merge 1 commit intocurl:masterfrom
jay:revert_getaddrinfoexw
Closed

asyn-thread: stop using GetAddrInfoExW on Windows#14794
jay wants to merge 1 commit intocurl:masterfrom
jay:revert_getaddrinfoexw

Conversation

@jay
Copy link
Member

@jay jay commented Sep 5, 2024

  • For the threaded resolver backend on Windows, revert back to exclusively use the threaded resolver with libcurl-owned threading instead of GetAddrInfoExW with Windows-owned threading.

Winsock (the Windows sockets library) has a bug where it does not wait for all of the name resolver threads it is managing to terminate before returning from WSACleanup. The threads continue to run and may cause a crash.

This commit is effectively a revert of several commits that encompass all GetAddrInfoExW code in libcurl. A manual review of merge conflicts was used to resolve minor changes that had modified the code for aesthetic or build reasons in other commits.

Prior to this change if libcurl was built with the threaded resolver backend for Windows, and Windows 8 or later was the operating system at runtime, and the caller was not impersonating another user, then libcurl would use GetAddrInfoExW to handle asynchronous name lookups.

GetAddrInfoExW support was added in a6bbc87, which preceded 8.6.0, and prior to that the threaded resolver backend used libcurl-owned threading exclusively on Windows.

Reported-by: Ionuț-Francisc Oancea
Reported-by: Razvan Pricope

Ref: https://developercommunity.visualstudio.com/t/ASAN:-heap-use-after-free-in-NdrFullPoin/10654169

Fixes #13509 (comment)
Closes #xxxx


Revert "asyn-thread: avoid using GetAddrInfoExW with impersonation"

This reverts commit 0caadc1.

Conflicts:
lib/system_win32.c

--

Revert "asyn-thread: fix curl_global_cleanup crash in Windows"

This reverts commit 428579f.

--

Revert "system_win32: fix a function pointer assignment warning"

This reverts commit 26f002e.

--

Revert "asyn-thread: use GetAddrInfoExW on >= Windows 8"

This reverts commit a6bbc87.

Conflicts:
lib/asyn-thread.c
lib/system_win32.c

--

- For the threaded resolver backend on Windows, revert back to
  exclusively use the threaded resolver with libcurl-owned threading
  instead of GetAddrInfoExW with Windows-owned threading.

Winsock (the Windows sockets library) has a bug where it does not wait
for all of the name resolver threads it is managing to terminate before
returning from WSACleanup. The threads continue to run and may cause a
crash.

This commit is effectively a revert of several commits that encompass
all GetAddrInfoExW code in libcurl. A manual review of merge conflicts
was used to resolve minor changes that had modified the code for
aesthetic or build reasons in other commits.

Prior to this change if libcurl was built with the threaded resolver
backend for Windows, and Windows 8 or later was the operating system at
runtime, and the caller was not impersonating another user, then libcurl
would use GetAddrInfoExW to handle asynchronous name lookups.

GetAddrInfoExW support was added in a6bbc87, which preceded 8.6.0, and
prior to that the threaded resolver backend used libcurl-owned threading
exclusively on Windows.

Reported-by: Ionuț-Francisc Oancea
Reported-by: Razvan Pricope

Ref: https://developercommunity.visualstudio.com/t/ASAN:-heap-use-after-free-in-NdrFullPoin/10654169

Fixes curl#13509 (comment)
Closes #xxxx

---

Revert "asyn-thread: avoid using GetAddrInfoExW with impersonation"

This reverts commit 0caadc1.

Conflicts:
	lib/system_win32.c

--

Revert "asyn-thread: fix curl_global_cleanup crash in Windows"

This reverts commit 428579f.

--

Revert "system_win32: fix a function pointer assignment warning"

This reverts commit 26f002e.

--

Revert "asyn-thread: use GetAddrInfoExW on >= Windows 8"

This reverts commit a6bbc87.

Conflicts:
	lib/asyn-thread.c
	lib/system_win32.c

--
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

Unfortunate but necessary.

@jay jay closed this in eb8ad66 Sep 8, 2024
@jay jay deleted the revert_getaddrinfoexw branch September 8, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

Windows DNS resolution: Curl crash when GetAddrInfoExW callback invoked on shutdown

2 participants