Skip to content

wslcsession: use DuplicateHandle idiom for self-process handle in GetProcessHandle#40748

Open
benhillis wants to merge 1 commit into
masterfrom
fix/wslc-getprocesshandle-impersonation
Open

wslcsession: use DuplicateHandle idiom for self-process handle in GetProcessHandle#40748
benhillis wants to merge 1 commit into
masterfrom
fix/wslc-getprocesshandle-impersonation

Conversation

@benhillis

@benhillis benhillis commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary of the Pull Request

Replace the self-OpenProcess(GetCurrentProcessId()) calls in IWSLCSessionFactory::GetProcessHandle and IWSLCSession::GetProcessHandle with a duplicate of the current-process pseudo-handle via the existing wslutil::DuplicateHandle helper.

This is a cleanup/hardening change, not a behavioral one. A process should not need to OpenProcess itself by PID to obtain a handle to itself — duplicating the GetCurrentProcess() pseudo-handle is the canonical idiom. The new call requests the same access rights (PROCESS_SET_QUOTA | PROCESS_TERMINATE), so the marshaled handle and all downstream usage are unchanged.

Note

This does not fix the reported 0x80070005 (E_ACCESSDENIED) seen after hibernate/resume. Debugging shows that failure originates from AssignProcessToJobObject in wslservice.exe (WSLCSessionManager::AddSessionProcessToJobObject, WSLCSessionManager.cpp:454), not from GetProcessHandle. Both call sites surface the identical HRESULT, which is why it was initially misattributed. The job-assignment failure is being tracked separately.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already
  • Tests: N/A — no behavioral change (handle access rights are identical)
  • Localization: No end-user-facing strings
  • Dev docs: N/A

Detailed Description of the Pull Request / Additional comments

GetProcessHandle previously did:

wil::unique_handle process{OpenProcess(PROCESS_SET_QUOTA | PROCESS_TERMINATE, FALSE, GetCurrentProcessId())};

OpenProcess performs an access check against the calling thread's effective token. Using it to open your own process is unnecessary and fragile — there is no reason a process should need an access check to obtain a handle to itself.

The replacement duplicates the current-process pseudo-handle:

*ProcessHandle = wslutil::DuplicateHandle(GetCurrentProcess(), PROCESS_SET_QUOTA | PROCESS_TERMINATE);

DuplicateHandle from the pseudo-handle performs no object DACL access check (it only requires PROCESS_DUP_HANDLE on the GetCurrentProcess() pseudo-handle, which is always granted), so it is the correct, robust idiom. Because an explicit desired-access mask is passed, the resulting handle carries exactly PROCESS_SET_QUOTA | PROCESS_TERMINATE — identical to the previous handle.

Validation Steps Performed

  • Build.
  • Verified the duplicated handle carries the same access rights (PROCESS_SET_QUOTA | PROCESS_TERMINATE), so AssignProcessToJobObject consumption is unchanged.

Copilot AI review requested due to automatic review settings June 8, 2026 19:33

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

Fixes intermittent E_ACCESSDENIED when WSLC exposes a handle to its own COM server process under COM impersonation by avoiding OpenProcess() (which performs an access check against the thread’s effective token) and instead duplicating the current-process pseudo-handle.

Changes:

  • Replace OpenProcess(PROCESS_SET_QUOTA | PROCESS_TERMINATE, ...) with wslutil::DuplicateHandle(GetCurrentProcess(), ...) in WSLC session and factory GetProcessHandle implementations.
  • Add an explicit E_POINTER guard in WSLCSessionFactory::GetProcessHandle.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/windows/wslcsession/WSLCSessionFactory.cpp Switch self-process handle creation to wslutil::DuplicateHandle to avoid impersonation-related access checks.
src/windows/wslcsession/WSLCSession.cpp Switch self-process handle creation to wslutil::DuplicateHandle to avoid impersonation-related access checks.

Comment thread src/windows/wslcsession/WSLCSessionFactory.cpp Outdated
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Copilot AI review requested due to automatic review settings June 9, 2026 01:35

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

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

Comment thread src/windows/wslcsession/WSLCSessionFactory.cpp Outdated
@benhillis benhillis marked this pull request as ready for review June 9, 2026 15:42
@benhillis benhillis requested a review from a team as a code owner June 9, 2026 15:42
Copilot AI review requested due to automatic review settings June 9, 2026 15:42

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

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

@OneBlue OneBlue left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FYI debugging showed that the error wasn't returned from OpenProcess(), but from AssignProcessToJobObject() in wslservice.exe, so I'm not sure that this will solve the issue

@benhillis benhillis changed the title Fix WSLC GetProcessHandle E_ACCESSDENIED under COM impersonation wslcsession: use DuplicateHandle idiom for self-process handle in GetProcessHandle Jun 10, 2026
…ProcessHandle

Replace OpenProcess(GetCurrentProcessId()) in IWSLCSessionFactory::GetProcessHandle
and IWSLCSession::GetProcessHandle with a duplicate of the GetCurrentProcess()
pseudo-handle via wslutil::DuplicateHandle.

A process should not need an access check to obtain a handle to itself; duplicating
the pseudo-handle is the canonical idiom and avoids OpenProcess's access check against
the calling thread's (possibly impersonated) effective token. An explicit desired-access
mask keeps the handle's rights identical (PROCESS_SET_QUOTA | PROCESS_TERMINATE), so the
marshaled handle and downstream usage are unchanged.

This is a cleanup/hardening change. It does not fix the 0x80070005 seen after
hibernate/resume, which originates from AssignProcessToJobObject in wslservice.exe
(WSLCSessionManager::AddSessionProcessToJobObject), not from GetProcessHandle.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@benhillis benhillis force-pushed the fix/wslc-getprocesshandle-impersonation branch from 753c768 to 2e4172b Compare June 10, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants