Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions localization/strings/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -2361,6 +2361,10 @@ For privacy information about this product please visit https://aka.ms/privacy.<
<value>No WSLC session found in '{}'</value>
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="MessageWslcSessionStorageMustBeEmpty" xml:space="preserve">
<value>Cannot use '{}' as session storage because the directory is not empty</value>
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
</data>
<data name="MessageWslcTagImageInvalidFormat" xml:space="preserve">
<value>Invalid image tag format: '{}'. Expected format is 'name:tag'</value>
<comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment>
Expand Down
18 changes: 17 additions & 1 deletion src/windows/service/exe/WSLCSessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,18 @@ void WSLCSessionManagerImpl::CreateSession(
const auto sessionSettings = CreateSessionSettings(sessionId, creatorPid, Settings, resolvedDisplayName.c_str());
wil::com_ptr<IWSLCSession> session;
wil::com_ptr<IWSLCSessionReference> serviceRef;
THROW_IF_FAILED(factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), WarningCallback, &session, &serviceRef));
{
const auto factoryHr = factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), WarningCallback, &session, &serviceRef);
if (FAILED(factoryHr))
{
if (auto comError = wslutil::GetCOMErrorInfo(); comError && comError->Message)
{
THROW_HR_WITH_USER_ERROR(factoryHr, comError->Message.get());
}

THROW_IF_FAILED(factoryHr);
}
}

// Track the session via its service ref, along with metadata and security info.
m_sessions.push_back(SessionEntry{
Expand Down Expand Up @@ -591,6 +602,11 @@ HRESULT WSLCSessionManager::OpenSessionByName(_In_ LPCWSTR DisplayName, _Out_ IW
return CallImpl(&WSLCSessionManagerImpl::OpenSessionByName, DisplayName, Session);
}

HRESULT WSLCSessionManager::InterfaceSupportsErrorInfo(_In_ REFIID riid)
{
return riid == __uuidof(IWSLCSessionManager) ? S_OK : S_FALSE;
}

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

WSLCSessionManagerImpl* WSLCSessionManagerImpl::Instance() noexcept
Expand Down
5 changes: 4 additions & 1 deletion src/windows/service/exe/WSLCSessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class WSLCSessionManagerImpl
} // namespace wsl::windows::service::wslc

class DECLSPEC_UUID("a9b7a1b9-0671-405c-95f1-e0612cb4ce8f") WSLCSessionManager
: public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>, IWSLCSessionManager, IFastRundown>,
: public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>, IWSLCSessionManager, IFastRundown, ISupportErrorInfo>,
public wsl::windows::service::wslc::COMImplClass<wsl::windows::service::wslc::WSLCSessionManagerImpl>
{
public:
Expand All @@ -208,4 +208,7 @@ class DECLSPEC_UUID("a9b7a1b9-0671-405c-95f1-e0612cb4ce8f") WSLCSessionManager
IFACEMETHOD(ListSessions)(_Out_ WSLCSessionListEntry** Sessions, _Out_ ULONG* SessionsCount) override;
IFACEMETHOD(OpenSession)(_In_ ULONG Id, _Out_ IWSLCSession** Session) override;
IFACEMETHOD(OpenSessionByName)(_In_ LPCWSTR DisplayName, _Out_ IWSLCSession** Session) override;

// ISupportErrorInfo: enables IErrorInfo marshaling across COM boundaries.
IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override;
};
58 changes: 49 additions & 9 deletions src/windows/wslcsession/WSLCSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ using wsl::windows::service::wslc::WSLCVirtualMachine;
constexpr auto c_containerdStorage = "/var/lib/docker";
constexpr auto c_containerdSocket = "/run/containerd/containerd.sock";
constexpr auto c_dockerdReadyLogLine = "API listen on /var/run/docker.sock";
constexpr auto c_storageVhdFilename = L"storage.vhdx";
constexpr DWORD c_processTerminateTimeoutMs = 30 * 1000;
constexpr DWORD c_processKillTimeoutMs = 10 * 1000;

Expand Down Expand Up @@ -285,6 +286,52 @@ try
TraceLoggingValue(m_displayName.c_str(), "DisplayName"),
TraceLoggingValue(Settings->CreatorPid, "CreatorPid"));

// Validate storage path before creating VM resources (Terminate() would clobber the error).
if (Settings->StoragePath != nullptr)
{
std::filesystem::path storagePath{Settings->StoragePath};
THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(Settings->StoragePath), !storagePath.is_absolute());

// MSVC's std::filesystem sets ec to ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND
// for non-existent paths. Filter those out — only throw on real failures.
auto throwOnRealFsError = [](const std::error_code& ec, const std::filesystem::path& path, const char* op) {
if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND)
{
THROW_IF_WIN32_ERROR_MSG(ec.value(), "%hs failed for %ls", op, path.c_str());
}
};

std::error_code ec;
const auto vhdPath = storagePath / c_storageVhdFilename;
const bool vhdExists = std::filesystem::exists(vhdPath, ec);
throwOnRealFsError(ec, vhdPath, "exists");

Comment thread
beena352 marked this conversation as resolved.
THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND),
Localization::MessageWslcSessionStorageNotFound(Settings->StoragePath),
!vhdExists && WI_IsFlagSet(Settings->StorageFlags, WSLCSessionStorageFlagsNoCreate));

// Only check emptiness for new sessions. Existing sessions are identified by their VHD.
if (!vhdExists)
{
const bool isDir = std::filesystem::is_directory(storagePath, ec);
throwOnRealFsError(ec, storagePath, "is_directory");

if (isDir)
{
const bool empty = std::filesystem::is_empty(storagePath, ec);
THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_empty failed for %ls", storagePath.c_str());

THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(storagePath.c_str()), !empty);
}
else
{
std::filesystem::create_directories(storagePath, ec);
THROW_IF_WIN32_ERROR_MSG(ec.value(), "create_directories failed for %ls", storagePath.c_str());
}
Comment thread
beena352 marked this conversation as resolved.
Comment thread
beena352 marked this conversation as resolved.
}
}

// Create the VM.
m_virtualMachine.emplace(Vm, Settings, m_sessionTerminatingEvent.get());

Expand Down Expand Up @@ -356,10 +403,9 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID
return;
}

// Storage path validation is done in Initialize() before any VM resources are created.
std::filesystem::path storagePath{Settings.StoragePath};
THROW_HR_WITH_USER_ERROR_IF(E_INVALIDARG, Localization::MessagePathNotAbsolute(Settings.StoragePath), !storagePath.is_absolute());

m_storageVhdPath = storagePath / "storage.vhdx";
m_storageVhdPath = storagePath / c_storageVhdFilename;

std::string diskDevice;
std::optional<ULONG> diskLun{};
Expand Down Expand Up @@ -388,15 +434,9 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID
"Failed to attach vhd: %ls",
m_storageVhdPath.c_str());

THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND),
Localization::MessageWslcSessionStorageNotFound(Settings.StoragePath),
WI_IsFlagSet(Settings.StorageFlags, WSLCSessionStorageFlagsNoCreate));

// If the VHD wasn't found, create it.
WSL_LOG("CreateStorageVhd", TraceLoggingValue(m_storageVhdPath.c_str(), "StorageVhdPath"));

std::filesystem::create_directories(storagePath);
wsl::core::filesystem::CreateVhd(m_storageVhdPath.c_str(), Settings.MaximumStorageSizeMb * _1MB, UserSid, false, false);
vhdCreated = true;
Comment thread
beena352 marked this conversation as resolved.

Expand Down
7 changes: 5 additions & 2 deletions src/windows/wslcsession/WSLCSessionFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ Module Name:
#include "WSLCSessionFactory.h"
#include "WSLCSession.h"
#include "WSLCSessionReference.h"
#include "wslutil.h"

namespace wslutil = wsl::windows::common::wslutil;
namespace wslc = wsl::windows::service::wslc;

void wslc::WSLCSessionFactory::SetDestructionCallback(std::function<void()>&& callback)
Expand Down Expand Up @@ -68,6 +66,11 @@ try
}
CATCH_RETURN()

HRESULT wslc::WSLCSessionFactory::InterfaceSupportsErrorInfo(_In_ REFIID riid)
{
return riid == __uuidof(IWSLCSessionFactory) ? S_OK : S_FALSE;
}

HRESULT wslc::WSLCSessionFactory::GetProcessHandle(_Out_ HANDLE* ProcessHandle)
try
{
Expand Down
5 changes: 4 additions & 1 deletion src/windows/wslcsession/WSLCSessionFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Module Name:
namespace wsl::windows::service::wslc {

class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory
: public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>, IWSLCSessionFactory, IFastRundown>
: public Microsoft::WRL::RuntimeClass<Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>, IWSLCSessionFactory, IFastRundown, ISupportErrorInfo>
{
public:
NON_COPYABLE(WSLCSessionFactory);
Expand All @@ -53,6 +53,9 @@ class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory

IFACEMETHOD(GetProcessHandle)(_Out_ HANDLE* ProcessHandle) override;

// ISupportErrorInfo: enables IErrorInfo marshaling across COM boundaries.
IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override;

private:
std::function<void()> m_destructionCallback;
};
Expand Down
20 changes: 20 additions & 0 deletions test/windows/WSLCTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,26 @@ class WSLCTests
wil::com_ptr<IWSLCSession> session;
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, nullptr, &session), E_INVALIDARG);
}

// Reject non-empty storage directory that doesn't contain a session VHD.
{
const auto storagePath = std::filesystem::temp_directory_path() /
std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64());
std::filesystem::create_directories(storagePath);
auto cleanup = wil::scope_exit([&]() {
std::error_code ignored;
std::filesystem::remove_all(storagePath, ignored);
});

std::ofstream{storagePath / L"userfile.txt"} << "data";

auto settings = GetDefaultSessionSettings(L"storage-not-empty");
const auto storagePathString = storagePath.wstring();
settings.StoragePath = storagePathString.c_str();
wil::com_ptr<IWSLCSession> session;
VERIFY_ARE_EQUAL(sessionManager->CreateSession(&settings, WSLCSessionFlagsNone, nullptr, &session), E_INVALIDARG);
ValidateCOMErrorMessage(std::format(L"Cannot use '{}' as session storage because the directory is not empty", storagePathString));
}
}

struct VmInfo
Expand Down
3 changes: 2 additions & 1 deletion test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ class WSLCE2ESessionEnterTests
WSLC_TEST_METHOD(WSLCE2E_SessionEnter_StoragePathNotFound)
{
auto result = RunWslc(L"system session enter does-not-exist");
const auto expectedPath = std::filesystem::absolute(L"does-not-exist").wstring();
result.Verify({
.Stderr = L"The system cannot find the path specified. \r\nError code: ERROR_PATH_NOT_FOUND\r\n",
.Stderr = std::format(L"No WSLC session found in '{}'\r\nError code: ERROR_PATH_NOT_FOUND\r\n", expectedPath),
.ExitCode = 1,
});
}
Expand Down