Fix MoveDistribution VHD ownership restore to reuse the resolved file handle - #41131
Merged
Ben Hillis (benhillis) merged 1 commit intoJul 22, 2026
Merged
Ben Hillis (benhillis) merged 1 commit into
Ben Hillis (benhillis) merged 1 commit into
Conversation
… handle setVhdOwner now opens the destination impersonated with READ_CONTROL, then uses ReOpenFile to upgrade the same handle to WRITE_OWNER while running as SYSTEM, instead of re-opening the path by name as SYSTEM. This avoids a second path resolution under elevated privileges. Also moves the rollback scope_exit registration earlier so a failure in setVhdOwner while moving to the new path correctly triggers rollback.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines LxssUserSessionImpl::MoveDistribution() to restore a moved distro VHD’s original owner using a handle-preserving flow: open the destination VHD while impersonating the caller, then upgrade that same file object to WRITE_OWNER under SYSTEM via ReOpenFile, avoiding a second path walk as SYSTEM. It also adjusts rollback ordering so failures during ownership restoration correctly revert the move before registry updates can strand the distro registration.
Changes:
- Update VHD ownership restoration to open as the caller (
READ_CONTROL) and thenReOpenFiletoWRITE_OWNERunder SYSTEM withSE_RESTORE_NAME, reusing the resolved file object. - Register the rollback
scope_exitbefore attempting ownership restoration, ensuring rollback occurs ifsetVhdOwner()fails. - Expand share flags used during ownership restoration to include
FILE_SHARE_DELETEfor the involved handles.
Blue (OneBlue)
approved these changes
Jul 22, 2026
Ben Hillis (benhillis)
deleted the
user/benhill/movedistribution-vhd-owner-handle
branch
July 22, 2026 21:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When moving a distro's VHD via wsl --manage --move , MoveDistribution restores the VHD's original owner after the move by re-opening the destination path by name as SYSTEM with WRITE_OWNER.
This changes setVhdOwner to instead open the destination impersonated as the caller with READ_CONTROL, then use ReOpenFile to upgrade that same handle to WRITE_OWNER while running as SYSTEM with SE_RESTORE_NAME, rather than re-resolving the path by name. ReOpenFile reuses the existing file object instead of walking the path again.
Also moves the rollback scope_exit registration earlier so a failure in setVhdOwner while moving to the new path correctly triggers rollback of the move and registry changes.
No behavior change for existing tests (MoveVhdOwnership, MoveVhdWithAdminOwner both pass unchanged)."