Various improvements to interactions with user-provided handles

src/windows/common/wslutil.cpp

@@ -411,13 +411,14 @@ std::wstring wsl::windows::common::wslutil::DownloadFileImpl(
     return newHandle;
 }
 
-[[nodiscard]] HANDLE wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(_In_ HANDLE Handle)
+[[nodiscard]] HANDLE wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(_In_ HANDLE Handle, _In_ std::optional<DWORD> DesiredAccess)
 {
     const wil::unique_handle caller = OpenCallingProcess(PROCESS_DUP_HANDLE);
     THROW_LAST_ERROR_IF(!caller);
 
     HANDLE newHandle;
-    THROW_IF_WIN32_BOOL_FALSE(::DuplicateHandle(caller.get(), Handle, GetCurrentProcess(), &newHandle, 0, FALSE, DUPLICATE_SAME_ACCESS));
+    THROW_IF_WIN32_BOOL_FALSE(::DuplicateHandle(
+        caller.get(), Handle, GetCurrentProcess(), &newHandle, DesiredAccess.value_or(0), FALSE, DesiredAccess.has_value() ? 0 : DUPLICATE_SAME_ACCESS));
 
     return newHandle;
 }

src/windows/common/wslutil.h

@@ -160,7 +160,7 @@ std::wstring DownloadFileImpl(std::wstring_view Url, std::wstring Filename, cons
 
 [[nodiscard]] HANDLE DuplicateHandle(_In_ HANDLE Handle, _In_ std::optional<DWORD> DesiredAccess = std::nullopt, _In_ BOOL InheritHandle = FALSE);
 
-[[nodiscard]] HANDLE DuplicateHandleFromCallingProcess(_In_ HANDLE Handle);
+[[nodiscard]] HANDLE DuplicateHandleFromCallingProcess(_In_ HANDLE Handle, _In_ std::optional<DWORD> DesiredAccess = {});
 
 [[nodiscard]] HANDLE DuplicateHandleToCallingProcess(_In_ HANDLE Handle, _In_ std::optional<DWORD> DesiredAccess = {});
 

src/windows/wslcsession/WSLCContainer.cpp

@@ -461,7 +461,9 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, ULONG* Stdin, ULONG* Stdout, U
     // If this is a TTY process, the PTY handle can be returned directly.
     if (WI_IsFlagSet(m_initProcessFlags, WSLCProcessFlagsTty))
     {
-        *Stdin = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(ioHandle.get())));
+        *Stdin = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(
+            reinterpret_cast<HANDLE>(ioHandle.get()), GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE));
+
         return;
     }
 
@@ -485,9 +487,14 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, ULONG* Stdin, ULONG* Stdout, U
 
     m_ioRelay.AddHandles(std::move(handles));
 
-    *Stdin = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stdinWrite.get())));
-    *Stdout = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stdoutRead.get())));
-    *Stderr = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stderrRead.get())));
+    *Stdin = HandleToULong(
+        common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stdinWrite.get()), GENERIC_WRITE | SYNCHRONIZE));
+
+    *Stdout = HandleToULong(
+        common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stdoutRead.get()), GENERIC_READ | SYNCHRONIZE));
+
+    *Stderr = HandleToULong(
+        common::wslutil::DuplicateHandleToCallingProcess(reinterpret_cast<HANDLE>(stderrRead.get()), GENERIC_READ | SYNCHRONIZE));
 }
 
 void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, LPCSTR DetachKeys)
@@ -724,7 +731,7 @@ void WSLCContainerImpl::Export(ULONG OutHandle) const
     std::pair<uint32_t, wil::unique_socket> SocketCodePair;
     SocketCodePair = m_dockerClient.ExportContainer(m_id);
 
-    wil::unique_handle containerFileHandle{wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(OutHandle))};
+    auto userHandle = m_wslcSession.OpenUserHandle(OutHandle, GENERIC_WRITE | SYNCHRONIZE);
 
     wsl::windows::common::relay::MultiHandleWait io = m_wslcSession.CreateIOContext();
 
@@ -741,8 +748,7 @@ void WSLCContainerImpl::Export(ULONG OutHandle) const
     else
     {
         io.AddHandle(
-            std::make_unique<RelayHandle<HTTPChunkBasedReadHandle>>(
-                HandleWrapper{std::move(SocketCodePair.second)}, HandleWrapper{std::move(containerFileHandle)}),
+            std::make_unique<RelayHandle<HTTPChunkBasedReadHandle>>(HandleWrapper{std::move(SocketCodePair.second)}, userHandle.Get()),
             wsl::windows::common::relay::MultiHandleWait::CancelOnCompleted);
     }
 
@@ -1305,7 +1311,7 @@ void WSLCContainerImpl::Logs(WSLCLogsFlags Flags, ULONG* Stdout, ULONG* Stderr,
         auto handle = std::make_unique<RelayHandle<HTTPChunkBasedReadHandle>>(std::move(socket), std::move(ttyWrite));
         m_ioRelay.AddHandle(std::move(handle));
 
-        *Stdout = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(ttyRead.get()));
+        *Stdout = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(ttyRead.get(), GENERIC_READ | SYNCHRONIZE));
     }
     else
     {
@@ -1318,8 +1324,8 @@ void WSLCContainerImpl::Logs(WSLCLogsFlags Flags, ULONG* Stdout, ULONG* Stderr,
 
         m_ioRelay.AddHandle(std::move(handle));
 
-        *Stdout = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(stdoutRead.get()));
-        *Stderr = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(stderrRead.get()));
+        *Stdout = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(stdoutRead.get(), GENERIC_READ | SYNCHRONIZE));
+        *Stderr = HandleToULong(common::wslutil::DuplicateHandleToCallingProcess(stderrRead.get(), GENERIC_READ | SYNCHRONIZE));
     }
 }
 

src/windows/wslcsession/WSLCSession.cpp

@@ -21,6 +21,7 @@ Module Name:
 using namespace wsl::windows::common;
 using relay::MultiHandleWait;
 using wsl::shared::Localization;
+using wsl::windows::service::wslc::UserHandle;
 using wsl::windows::service::wslc::WSLCSession;
 using wsl::windows::service::wslc::WSLCVirtualMachine;
 
@@ -97,6 +98,50 @@ wslc_schema::InspectImage ConvertInspectImage(const docker_schema::InspectImage&
 
 namespace wsl::windows::service::wslc {
 
+UserHandle::UserHandle(WSLCSession& Session, wil::unique_handle&& handle) : m_session(&Session), m_handle(std::move(handle))
+{
+    WI_ASSERT(!!m_handle);
+}
+
+UserHandle::UserHandle(UserHandle&& Other)
+{
+    *this = std::move(Other);
+}
+
+UserHandle& UserHandle::operator=(UserHandle&& Other)
+{
+    if (this != &Other)
+    {
+        Reset();
+        m_session = Other.m_session;
+        m_handle = std::move(Other.m_handle);
+
+        Other.m_session = nullptr;
+    }
+    return *this;
+}
+
+void UserHandle::Reset()
+{
+    if (m_handle)
+    {
+        WI_ASSERT(m_session != nullptr);
+
+        m_session->ReleaseUserHandle(m_handle.get());
+        m_handle.reset();
+    }
+}
+
+UserHandle::~UserHandle()
+{
+    Reset();
+}
+
+HANDLE UserHandle::Get() const noexcept
+{
+    return m_handle.get();
+}
+
 HRESULT WSLCSession::GetProcessHandle(_Out_ HANDLE* ProcessHandle)
 try
 {
@@ -406,10 +451,10 @@ try
     RETURN_HR_IF(E_INVALIDARG, Options->Tags.Count > 0 && Options->Tags.Values == nullptr);
     RETURN_HR_IF(E_INVALIDARG, Options->BuildArgs.Count > 0 && Options->BuildArgs.Values == nullptr);
 
-    wil::unique_handle dockerfileFileHandle;
+    std::optional<UserHandle> dockerfileFileHandle;
     if (Options->DockerfileHandle != 0 && Options->DockerfileHandle != HandleToULong(INVALID_HANDLE_VALUE))
     {
-        dockerfileFileHandle.reset(wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(Options->DockerfileHandle)));
+        dockerfileFileHandle = OpenUserHandle(Options->DockerfileHandle, GENERIC_READ | SYNCHRONIZE);
     }
 
     auto lock = m_lock.lock_shared();
@@ -438,11 +483,13 @@ try
         buildArgs.push_back("--build-arg");
         buildArgs.push_back(Options->BuildArgs.Values[i]);
     }
-    if (dockerfileFileHandle)
+
+    if (dockerfileFileHandle.has_value())
     {
         buildArgs.push_back("-f");
         buildArgs.push_back("-");
     }
+
     buildArgs.push_back(mountPath);
 
     WSL_LOG("BuildImageStart", TraceLoggingValue(wsl::shared::string::Join(buildArgs, ' ').c_str(), "Command"));
@@ -455,8 +502,7 @@ try
     if (dockerfileFileHandle)
     {
         io.AddHandle(std::make_unique<relay::RelayHandle<relay::ReadHandle>>(
-            common::relay::HandleWrapper{std::move(dockerfileFileHandle)},
-            common::relay::HandleWrapper{buildProcess.GetStdHandle(WSLCFDStdin)}));
+            common::relay::HandleWrapper{dockerfileFileHandle->Get()}, common::relay::HandleWrapper{buildProcess.GetStdHandle(WSLCFDStdin)}));
     }
 
     bool verbose = Options->Verbose;
@@ -598,7 +644,7 @@ CATCH_RETURN();
 
 void WSLCSession::ImportImageImpl(DockerHTTPClient::HTTPRequestContext& Request, ULONG InputHandle)
 {
-    wil::unique_handle imageFileHandle{wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(InputHandle))};
+    auto userHandle = OpenUserHandle(InputHandle, GENERIC_READ | SYNCHRONIZE);
 
     THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());
 
@@ -658,7 +704,7 @@ void WSLCSession::ImportImageImpl(DockerHTTPClient::HTTPRequestContext& Request,
     };
 
     io.AddHandle(std::make_unique<relay::RelayHandle<relay::ReadHandle>>(
-        common::relay::HandleWrapper{std::move(imageFileHandle)}, common::relay::HandleWrapper{Request.stream.native_handle()}));
+        userHandle.Get(), common::relay::HandleWrapper{Request.stream.native_handle()}));
 
     io.AddHandle(
         std::make_unique<DockerHTTPClient::DockerHttpResponseHandle>(Request, std::move(onHttpResponse), std::move(onProgress)),
@@ -699,7 +745,7 @@ CATCH_RETURN();
 
 void WSLCSession::SaveImageImpl(std::pair<uint32_t, wil::unique_socket>& SocketCodePair, ULONG OutputHandle, HANDLE CancelEvent)
 {
-    wil::unique_handle imageFileHandle{wsl::windows::common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(OutputHandle))};
+    auto userHandle = OpenUserHandle(OutputHandle, GENERIC_WRITE | SYNCHRONIZE);
 
     THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_dockerClient.has_value());
 
@@ -722,7 +768,7 @@ void WSLCSession::SaveImageImpl(std::pair<uint32_t, wil::unique_socket>& SocketC
     {
         io.AddHandle(
             std::make_unique<relay::RelayHandle<relay::HTTPChunkBasedReadHandle>>(
-                common::relay::HandleWrapper{std::move(SocketCodePair.second)}, common::relay::HandleWrapper{std::move(imageFileHandle)}),
+                common::relay::HandleWrapper{std::move(SocketCodePair.second)}, userHandle.Get()),
             MultiHandleWait::CancelOnCompleted);
     }
 
@@ -1398,9 +1444,19 @@ CATCH_RETURN();
 HRESULT WSLCSession::Terminate()
 try
 {
-    // m_sessionTerminatingEvent is always valid, so it can be signalled with the lock.
-    // This allows a session to be unblocked if a stuck operation is holding the lock.
-    m_sessionTerminatingEvent.SetEvent();
+
+    {
+        std::lock_guard lock(m_userHandlesLock);
+
+        // m_sessionTerminatingEvent is always valid, so it can be signalled without holding m_lock.
+        // This allows a session to be unblocked if a stuck operation is holding m_lock.
+        // N.B. This must happen under m_userHandlesLock to synchronize with potentially running operations.
+        m_sessionTerminatingEvent.SetEvent();
+
+        // Cancel any pending IO on user-provided handles to unblock operations
+        // in case the handles don't support overlapped IO.
+        CancelUserHandleIO();
+    }
 
     // Acquire an exclusive lock to ensure that no operation is running.
     auto lock = m_lock.lock_exclusive();
@@ -1597,6 +1653,46 @@ MultiHandleWait WSLCSession::CreateIOContext(HANDLE CancelHandle)
     return io;
 }
 
+UserHandle WSLCSession::OpenUserHandle(ULONG Handle, DWORD Access)
+{
+    std::lock_guard lock(m_userHandlesLock);
+
+    // Don't allow new handles to be added to the list if the session is terminating.
+    // N.B. This check must happen under m_userHandlesLock to synchronize with Terminate().
+
+    THROW_HR_IF_MSG(
+        E_ABORT, m_sessionTerminatingEvent.is_signaled(), "Refusing to open a user handle while the session is terminating.");
+
+    wil::unique_handle duplicatedHandle{common::wslutil::DuplicateHandleFromCallingProcess(ULongToHandle(Handle), Access)};
+
+    m_userHandles.emplace_back(duplicatedHandle.get());
+
+    return UserHandle{*this, std::move(duplicatedHandle)};
+}
+
+void WSLCSession::ReleaseUserHandle(HANDLE Handle)
+{
+    std::lock_guard lock(m_userHandlesLock);
+
+    auto it = std::ranges::find(m_userHandles, Handle);
+    WI_ASSERT(it != m_userHandles.end());
+
+    m_userHandles.erase(it);
+}
+
+void WSLCSession::CancelUserHandleIO()
+{
+    for (auto handle : m_userHandles)
+    {
+        // Cancel all IO on the handle.
+        // N.B. This only cancels IO happening in this process.
+        if (!CancelIoEx(handle, nullptr))
+        {
+            LOG_LAST_ERROR_IF(GetLastError() != ERROR_NOT_FOUND);
+        }
+    }
+}
+
 void WSLCSession::OnContainerDeleted(const WSLCContainerImpl* Container)
 {
     auto lock = m_lock.lock_shared();

src/windows/wslcsession/WSLCSession.h

@@ -28,6 +28,26 @@ namespace wsl::windows::service::wslc {
 
 class WSLCSession;
 
+class UserHandle
+{
+    NON_COPYABLE(UserHandle);
+
+public:
+    UserHandle(WSLCSession& Session, wil::unique_handle&& handle);
+    UserHandle(UserHandle&& Other);
+
+    ~UserHandle();
+
+    UserHandle& operator=(UserHandle&& Other);
+
+    HANDLE Get() const noexcept;
+    void Reset();
+
+private:
+    WSLCSession* m_session{};
+    wil::unique_handle m_handle;
+};
+
 //
 // WSLCSession - Implements IWSLCSession for container management.
 // Runs in a per-user COM server process for security isolation.
@@ -93,9 +113,13 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession
 
     common::relay::MultiHandleWait CreateIOContext(HANDLE CancelHandle = nullptr);
 
+    UserHandle OpenUserHandle(ULONG Handle, DWORD Access);
+    void ReleaseUserHandle(HANDLE Handle);
+
 private:
     ULONG m_id = 0;
 
+    __requires_lock_held(m_userHandlesLock) void CancelUserHandleIO();
     void ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID UserSid);
     void Ext4Format(const std::string& Device);
     void OnContainerDeleted(const WSLCContainerImpl* Container);
@@ -130,6 +154,10 @@ class DECLSPEC_UUID("4877FEFC-4977-4929-A958-9F36AA1892A4") WSLCSession
     std::function<void()> m_destructionCallback;
     std::atomic<bool> m_terminated{false};
 
+    // User-provided handles that the session is currently doing IO on.
+    std::mutex m_userHandlesLock;
+    __guarded_by(m_userHandlesLock) std::vector<HANDLE> m_userHandles;
+
     // Used for testing only.
     std::mutex m_allocatedPortsLock;
     __guarded_by(m_allocatedPortsLock) std::map<uint16_t, std::pair<std::shared_ptr<VmPortAllocation>, size_t>> m_allocatedPorts;