@samitouri / QOSAMI-WSL / commits / 77288148

Fix potential use-after-free on the container deletion path (#41018)

* Fix use-after-free when on the container deletion path * Add test coverage * Format * Apply PR feedback

Blue committed Jul 8, 2026 at 14:54 UTC 772881482e2cc2a7dd792d6375101338df5235f4

7 files changed +136 -29

src/windows/common/COMImplClass.h

+29 -13

@@ -18,12 +18,14 @@ Abstract:

19 namespace wsl::windows::service::wslc {

21 -template <typename TImpl>

21 +template <typename TImpl, typename TPointer = TImpl*>

22 class COMImplClass

23 {

24 public:

25 - COMImplClass(TImpl* impl) : m_impl(impl)

25 + void Initialize(TPointer impl)

26 {

27 + std::unique_lock lock(m_lock);

28 + m_impl = std::move(impl);

29 }

31 void Disconnect() noexcept

@@ -38,8 +40,7 @@ public:

40 return m_callers.empty() || m_callers.size() == 1 && *m_callers.begin() == std::this_thread::get_id();

41 });

41 - WI_ASSERT(m_impl != nullptr);

42 - m_impl = nullptr;

43 + m_impl = {};

44 }

46 protected:

@@ -48,7 +49,7 @@ protected:

49 try

50 {

51 auto [lock, impl] = LockImpl();

51 - (impl->*routine)(std::forward<Args>(args)...);

52 + ((*impl).*routine)(std::forward<Args>(args)...);

54 return S_OK;

55 }

@@ -59,22 +60,37 @@ protected:

60 try

61 {

62 auto [lock, impl] = LockImpl();

62 - (impl->*routine)(std::forward<Args>(args)...);

63 + ((*impl).*routine)(std::forward<Args>(args)...);

65 return S_OK;

66 }

67 CATCH_RETURN();

68 - [[nodiscard]] auto LockImpl()

69 + auto GetPointer()

70 {

70 - // Check if m_impl is available and add ourselves to the list of callers if that's the case.

71 + if constexpr (std::is_same_v<TPointer, TImpl*>)

72 + {

73 + return m_impl;

74 + }

75 + else

76 {

77 + return m_impl.lock();

78 + }

79 + }

80 +

81 + [[nodiscard]] auto LockImpl()

82 + {

83 + auto impl = [this] {

84 std::unique_lock lock{m_lock};

73 - THROW_HR_IF(RPC_E_DISCONNECTED, m_impl == nullptr);

85 +

86 + auto pointer = GetPointer();

87 + THROW_HR_IF(RPC_E_DISCONNECTED, !pointer);

89 auto [_, inserted] = m_callers.insert(std::this_thread::get_id());

90 WI_ASSERT(inserted);

77 - }

91 +

92 + return pointer;

93 + }();

95 auto release = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [this]() {

96 std::unique_lock lock{m_lock};

@@ -85,14 +101,14 @@ protected:

101 m_cv.notify_one();

102 });

103

88 - return std::make_pair(std::move(release), m_impl);

104 + return std::make_pair(std::move(release), std::move(impl));

105 }

106

107 private:

108 std::mutex m_lock;

109 std::condition_variable m_cv;

110 _Guarded_by_(m_lock) std::unordered_set<std::thread::id> m_callers;

95 - TImpl* m_impl = nullptr;

111 + TPointer m_impl{};

112 };

113

98 -} // namespace wsl::windows::service::wslc

\ No newline at end of file

114 +} // namespace wsl::windows::service::wslc

src/windows/service/exe/WSLCSessionManager.cpp

+2 -1

@@ -552,8 +552,9 @@ HRESULT WSLCSessionManagerImpl::CheckTokenAccess(const SessionEntry& Entry, cons

552 return S_OK;

553 }

554

555 -WSLCSessionManager::WSLCSessionManager(WSLCSessionManagerImpl* Impl) : COMImplClass<WSLCSessionManagerImpl>(Impl)

555 +WSLCSessionManager::WSLCSessionManager(WSLCSessionManagerImpl* Impl)

556 {

557 + Initialize(Impl);

558 }

559

560 HRESULT WSLCSessionManager::GetVersion(_Out_ WSLCVersion* Version)

src/windows/wslcsession/WSLCContainer.cpp

+18 -8

@@ -559,7 +559,7 @@ WSLCContainerImpl::WSLCContainerImpl(

559 m_volumes(Volumes),

560 m_mappedPorts(std::move(ports)),

561 m_labels(std::move(labels)),

562 - m_comWrapper(wil::MakeOrThrow<WSLCContainer>(this, wslcSession, std::move(onDeleted))),

562 + m_comWrapper(wil::MakeOrThrow<WSLCContainer>(wslcSession, std::move(onDeleted))),

563 m_dockerClient(DockerClient),

564 m_eventTracker(EventTracker),

565 m_ioRelay(Relay),

@@ -617,6 +617,12 @@ WSLCContainerImpl::~WSLCContainerImpl()

617 }

618 }

619

620 +void WSLCContainerImpl::Initialize()

621 +{

622 + // N.B. this must be done here because weak_from_this() is only valid after the constructor returns.

623 + m_comWrapper->Initialize(weak_from_this());

624 +}

625 +

626 void WSLCContainerImpl::SetExitCode(int ExitCode) noexcept

627 {

628 std::lock_guard processesLock{m_processesLock};

@@ -1384,7 +1390,7 @@ WslcInspectContainer WSLCContainerImpl::BuildInspectContainer(const DockerInspec

1390 return wslcInspect;

1391 }

1392

1387 -std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Create(

1393 +std::shared_ptr<WSLCContainerImpl> WSLCContainerImpl::Create(

1394 const WSLCContainerOptions& containerOptions,

1395 const std::string& containerName,

1396 WSLCSession& wslcSession,

@@ -1777,7 +1783,7 @@ std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Create(

1783 namedVolumes.emplace_back(containerOptions.NamedVolumes[i].Name);

1784 }

1785

1780 - auto container = std::make_unique<WSLCContainerImpl>(

1786 + auto container = std::make_shared<WSLCContainerImpl>(

1787 wslcSession,

1788 virtualMachine,

1789 pluginNotifier,

@@ -1799,11 +1805,13 @@ std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Create(

1805 containerOptions.InitProcessOptions.Flags,

1806 containerOptions.Flags);

1807

1808 + container->Initialize();

1809 +

1810 deleteOnFailure.release();

1811 return container;

1812 }

1813

1806 -std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Open(

1814 +std::shared_ptr<WSLCContainerImpl> WSLCContainerImpl::Open(

1815 const common::docker_schema::ContainerInfo& dockerContainer,

1816 WSLCSession& wslcSession,

1817 WSLCVirtualMachine& virtualMachine,

@@ -1869,7 +1877,7 @@ std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Open(

1877 }

1878 }

1879

1872 - auto container = std::make_unique<WSLCContainerImpl>(

1880 + auto container = std::make_shared<WSLCContainerImpl>(

1881 wslcSession,

1882 virtualMachine,

1883 pluginNotifier,

@@ -1891,6 +1899,8 @@ std::unique_ptr<WSLCContainerImpl> WSLCContainerImpl::Open(

1899 metadata.InitProcessFlags,

1900 metadata.Flags);

1901

1902 + container->Initialize();

1903 +

1904 // Restore the state change timestamp from Docker inspect data.

1905 try

1906 {

@@ -2179,8 +2189,8 @@ __requires_lock_held(m_lock) void WSLCContainerImpl::Transition(WSLCContainerSta

2189 m_stateChangedAt = stateChangedAt.value_or(static_cast<std::uint64_t>(std::time(nullptr)));

2190 }

2191

2182 -WSLCContainer::WSLCContainer(WSLCContainerImpl* impl, WSLCSession& session, std::function<void(const WSLCContainerImpl*)>&& OnDeleted) :

2183 - COMImplClass<WSLCContainerImpl>(impl), m_session(session), m_onDeleted(std::move(OnDeleted))

2192 +WSLCContainer::WSLCContainer(WSLCSession& session, std::function<void(const WSLCContainerImpl*)>&& OnDeleted) :

2193 + m_session(session), m_onDeleted(std::move(OnDeleted))

2194 {

2195 }

2196

@@ -2325,7 +2335,7 @@ try

2335 auto [lock, impl] = LockImpl();

2336

2337 impl->Delete(Flags);

2328 - m_onDeleted(impl);

2338 + m_onDeleted(impl.get());

2339

2340 return S_OK;

2341 }

src/windows/wslcsession/WSLCContainer.h

+7 -5

@@ -64,7 +64,7 @@ struct ContainerPortMapping

64 uint16_t ContainerPort{};

65 };

67 -class WSLCContainerImpl

67 +class WSLCContainerImpl : public std::enable_shared_from_this<WSLCContainerImpl>

68 {

69 public:

70 NON_COPYABLE(WSLCContainerImpl);

@@ -94,6 +94,8 @@ public:

95 ~WSLCContainerImpl();

97 + void Initialize();

98 +

99 void Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions);

100 void Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* Stdout, WSLCHandle* Stderr) const;

101 void Stop(_In_ WSLCSignal Signal, _In_ LONG TimeoutSeconds, bool Kill);

@@ -129,7 +131,7 @@ public:

131 return m_containerFlags;

132 }

133

132 - static std::unique_ptr<WSLCContainerImpl> Create(

134 + static std::shared_ptr<WSLCContainerImpl> Create(

135 const WSLCContainerOptions& Options,

136 const std::string& Name,

137 WSLCSession& wslcSession,

@@ -142,7 +144,7 @@ public:

144 DockerHTTPClient& DockerClient,

145 IORelay& Relay);

146

145 - static std::unique_ptr<WSLCContainerImpl> Open(

147 + static std::shared_ptr<WSLCContainerImpl> Open(

148 const common::docker_schema::ContainerInfo& DockerContainer,

149 WSLCSession& wslcSession,

150 WSLCVirtualMachine& virtualMachine,

@@ -224,11 +226,11 @@ private:

226

227 class DECLSPEC_UUID("B1F1C4E3-C225-4CAE-AD8A-34C004DE1AE4") WSLCContainer

228 : public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>, IWSLCContainer, IWSLCCompatContainer, IFastRundown, ISupportErrorInfo>,

227 - public COMImplClass<WSLCContainerImpl>

229 + public COMImplClass<WSLCContainerImpl, std::weak_ptr<WSLCContainerImpl>>

230 {

231

232 public:

231 - WSLCContainer(WSLCContainerImpl* impl, WSLCSession& session, std::function<void(const WSLCContainerImpl*)>&& OnDeleted);

233 + WSLCContainer(WSLCSession& session, std::function<void(const WSLCContainerImpl*)>&& OnDeleted);

234

235 IFACEMETHOD(Attach)(_In_opt_ LPCSTR DetachKeys, _Out_ WSLCHandle* Stdin, _Out_ WSLCHandle* Stdout, _Out_ WSLCHandle* Stderr) override;

236 IFACEMETHOD(Stop)(_In_ WSLCSignal Signal, _In_ LONG TimeoutSeconds) override;

src/windows/wslcsession/WSLCSession.cpp

+3 -1

@@ -3411,7 +3411,9 @@ void WSLCSession::OnContainerDeleted(const WSLCContainerImpl* Container)

3411 auto lock = m_lock.lock_shared();

3412 std::lock_guard containersLock(m_containersLock);

3413

3414 - WI_VERIFY(m_containers.erase(Container->ID()) == 1);

3414 + // N.B. once a container transitions to a 'Deleted' state, a call to ListContainers() can remove it from m_containers.

3415 + // Therefore it's possible that the container is already removed when the callback from Delete() is invoked.

3416 + m_containers.erase(Container->ID());

3417 }

3418

3419 HRESULT WSLCSession::GetState(_Out_ WSLCSessionState* State)

src/windows/wslcsession/WSLCSession.h

+1 -1

@@ -315,7 +315,7 @@ private:

315 // This allows independent operations to proceed while container bookkeeping remains synchronized.

316 // WSLCVolumes has its own internal srwlock and does not require m_lock.

317 std::mutex m_containersLock;

318 - std::unordered_map<std::string, std::unique_ptr<WSLCContainerImpl>> m_containers;

318 + std::unordered_map<std::string, std::shared_ptr<WSLCContainerImpl>> m_containers;

319 std::optional<WSLCVolumes> m_volumes;

320 std::mutex m_networksLock;

321 std::unordered_map<std::string, NetworkEntry> m_networks;

test/windows/WSLCTests.cpp

+76

@@ -6773,6 +6773,82 @@ class WSLCTests

6773 }

6774 }

6775

6776 + WSLC_TEST_METHOD(ContainerListDeleteStressTest)

6777 + {

6778 + constexpr auto c_iterations = 50;

6779 +

6780 + const std::string containerName = "wslc-list-delete-stress";

6781 +

6782 + std::atomic<unsigned int> failures = 0;

6783 +

6784 + // One thread repeatedly creates a container and then deletes it.

6785 + std::thread thread([&]() {

6786 + for (unsigned int i = 0; i < c_iterations; ++i)

6787 + {

6788 + WSLCContainerLauncher launcher("debian:latest", containerName, {"sleep", "99999"});

6789 +

6790 + auto [hrCreate, container] = launcher.CreateNoThrow(*m_defaultSession);

6791 + if (FAILED(hrCreate))

6792 + {

6793 + LogError("CreateContainer(%hs) unexpected HR: 0x%08x", containerName.c_str(), hrCreate);

6794 + ++failures;

6795 + continue;

6796 + }

6797 +

6798 + if (i % 2 == 0)

6799 + {

6800 + auto result = container->Get().Start(WSLCContainerStartFlagsNone, nullptr, nullptr);

6801 + if (FAILED(result))

6802 + {

6803 + LogError("Start(%hs) failed: 0x%08x", containerName.c_str(), result);

6804 + ++failures;

6805 + }

6806 +

6807 + if (i % 4 == 0)

6808 + {

6809 + result = container->Get().Stop(WSLCSignalSIGKILL, 0);

6810 + if (FAILED(result))

6811 + {

6812 + LogError("Stop(%hs) failed: 0x%08x", containerName.c_str(), result);

6813 + ++failures;

6814 + }

6815 + }

6816 + }

6817 +

6818 + HRESULT result = container->Get().Delete(WSLCDeleteFlagsForce);

6819 + if (FAILED(result))

6820 + {

6821 + LogError("Delete(%hs) failed: 0x%08x", containerName.c_str(), result);

6822 + ++failures;

6823 + }

6824 + else

6825 + {

6826 + container->SetDeleteOnClose(false);

6827 + }

6828 + }

6829 + });

6830 +

6831 + while (WaitForSingleObject(thread.native_handle(), 0) == WAIT_TIMEOUT)

6832 + {

6833 + WSLCListContainersOptions options{};

6834 + options.Flags = WSLCListContainersFlagsAll;

6835 +

6836 + wil::unique_cotaskmem_array_ptr<WSLCContainerEntry> containers;

6837 + wil::unique_cotaskmem_array_ptr<WSLCContainerPortMapping> ports;

6838 + HRESULT hrList = m_defaultSession->ListContainers(

6839 + &options, &containers, containers.size_address<ULONG>(), &ports, ports.size_address<ULONG>());

6840 + if (FAILED(hrList))

6841 + {

6842 + LogError("ListContainers unexpected HR: 0x%08x", hrList);

6843 + ++failures;

6844 + }

6845 + }

6846 +

6847 + thread.join();

6848 +

6849 + VERIFY_ARE_EQUAL(failures.load(), 0u);

6850 + }

6851 +

6852 WSLC_TEST_METHOD(ContainerNetwork)

6853 {

6854 auto expectContainerList = [&](const std::vector<std::tuple<std::string, std::string, WSLCContainerState>>& expectedContainers) {