Skip to content

windows: determine RtlVerifyVersionInfo address on global init#20853

Closed
vszakats wants to merge 8 commits intocurl:masterfrom
vszakats:wininit
Closed

windows: determine RtlVerifyVersionInfo address on global init#20853
vszakats wants to merge 8 commits intocurl:masterfrom
vszakats:wininit

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Mar 8, 2026

Instead of the first internal call to curlx_verify_windows_version().

To avoid the chance of a race, potentially resulting in initializing
this address twice. AFAICT it could not cause an issue before this
patch.

Reported by Codex Security

Follow-up to b17ef87 #18009


@vszakats vszakats marked this pull request as draft March 8, 2026 15:12
@vszakats vszakats marked this pull request as ready for review March 8, 2026 16:23
@vszakats vszakats requested a review from Copilot March 8, 2026 16:31
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 8, 2026

@aisle-analyzer

@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Mar 8, 2026

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

Aisle supplements but does not replace security review.


Analyzed PR: #20853 at commit 901a571

Last updated on: 2026-03-08T16:50:57Z

@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 8, 2026

augment review

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR moves Windows RtlVerifyVersionInfo function-pointer resolution to global initialization time (instead of lazily on first curlx_verify_windows_version() call), aiming to avoid a potential race during first use.

Changes:

  • Add curlx_verify_windows_init() and cache RtlVerifyVersionInfo in a file-scope static.
  • Invoke curlx_verify_windows_init() and curlx_now_init() from Curl_win32_init().
  • Remove the previous “one-time” lazy initialization logic from curlx_verify_windows_version().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
lib/system_win32.c Calls the new Windows-version init and centralizes QPC frequency init via curlx_now_init().
lib/curlx/version_win32.h Exposes curlx_verify_windows_init() (noop on UWP) for early initialization.
lib/curlx/version_win32.c Implements early init + switches callers to use the cached s_pRtlVerifyVersionInfo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 8, 2026

🤖 Augment PR Summary

Summary: Moves Windows version-check initialization into Win32 global init to avoid potential first-use races when resolving RtlVerifyVersionInfo.

Changes:

  • Add `curlx_verify_windows_init()` (no-op on UWP) and cache the `RtlVerifyVersionInfo` function pointer at file scope.
  • Remove lazy, per-call initialization from `curlx_verify_windows_version()` and use the cached pointer when available.
  • Call `curlx_verify_windows_init()` from `Curl_win32_init()` and switch QPC frequency setup to `curlx_now_init()`.

Technical Notes: Falls back to VerifyVersionInfoW when RtlVerifyVersionInfo cannot be resolved.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vszakats vszakats changed the title windows: initialize RtlVerifyVersionInfo address on global init windows: determine RtlVerifyVersionInfo address on global init Mar 8, 2026
@vszakats vszakats closed this in 6a68264 Mar 9, 2026
@vszakats vszakats deleted the wininit branch March 9, 2026 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants