Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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 @@ -272,7 +272,18 @@ void WSLCSessionManagerImpl::CreateSession(const WSLCSessionSettings* Settings,
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(), &session, &serviceRef));
{
const auto factoryHr = factory->CreateSession(&sessionSettings, vm.Get(), notifier.Get(), &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 @@ -588,6 +599,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 @@ -187,7 +187,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 @@ -203,4 +203,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;
};
82 changes: 74 additions & 8 deletions src/windows/wslcsession/WSLCSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,60 @@ std::string GenerateContainerName(int retry)
return name;
}

// Zero-byte sentinel written at the root of a session storage directory so we can
// distinguish a directory we own from one the user is using for something else.
constexpr auto c_sessionMarkerFile = L"wslcsession";

// Validates StoragePath and stamps the marker if needed.
// IsExistingStorage = true when a session VHD was just attached (legacy directories
// pre-dating the marker are tolerated and upgraded). When false the directory must
// be empty or non-existent so we never overwrite unrelated user files.
void EnsureSessionMarker(const std::filesystem::path& StoragePath, bool IsExistingStorage)
{
const auto markerPath = StoragePath / c_sessionMarkerFile;

const auto markerAttrs = GetFileAttributesW(markerPath.c_str());
if (markerAttrs != INVALID_FILE_ATTRIBUTES)
{
// A directory at the marker name means the storage path is used for something else.
THROW_HR_WITH_USER_ERROR_IF(
E_INVALIDARG, Localization::MessageWslcSessionStorageMustBeEmpty(StoragePath.c_str()), WI_IsFlagSet(markerAttrs, FILE_ATTRIBUTE_DIRECTORY));

return;
}

std::error_code ec;
const bool isDir = std::filesystem::is_directory(StoragePath, ec);

// is_directory sets ec when the path does not exist (ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND).
// That is the normal case for new sessions — only throw on unexpected errors.
if (ec && ec.value() != ERROR_FILE_NOT_FOUND && ec.value() != ERROR_PATH_NOT_FOUND)
{
THROW_IF_WIN32_ERROR_MSG(ec.value(), "is_directory failed for %ls", StoragePath.c_str());
}

Comment thread
beena352 marked this conversation as resolved.
Outdated
if (isDir)
{
if (!IsExistingStorage)
{
// New session into an existing directory: require it to be empty.
const bool empty = std::filesystem::is_empty(StoragePath, ec);
if (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);
}

wil::unique_hfile marker{CreateFileW(markerPath.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)};
THROW_LAST_ERROR_IF_MSG(!marker, "Failed to create marker file: %ls", markerPath.c_str());
}

} // namespace

namespace wsl::windows::service::wslc {
Expand Down Expand Up @@ -278,6 +332,25 @@ try
TraceLoggingValue(m_displayName.c_str(), "DisplayName"),
TraceLoggingValue(Settings->CreatorPid, "CreatorPid"));

// Validate the storage path before creating VM resources so rejection errors
// propagate cleanly (Terminate()'s cleanup can overwrite the error context).
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());

std::error_code ec;
const bool vhdExists = std::filesystem::exists(storagePath / L"storage.vhdx", ec);

Comment thread
beena352 marked this conversation as resolved.
// Reject up-front if the caller forbade creating new storage and there is no VHD to attach.
THROW_HR_WITH_USER_ERROR_IF(
HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND),
Localization::MessageWslcSessionStorageNotFound(Settings->StoragePath),
!vhdExists && WI_IsFlagSet(Settings->StorageFlags, WSLCSessionStorageFlagsNoCreate));

EnsureSessionMarker(storagePath, /*IsExistingStorage=*/vhdExists);
}

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

Expand Down Expand Up @@ -349,9 +422,8 @@ void WSLCSession::ConfigureStorage(const WSLCSessionInitSettings& Settings, PSID
return;
}

// Storage path validation and marker stamping are done in Initialize().
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";

std::string diskDevice;
Expand Down Expand Up @@ -381,15 +453,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
18 changes: 15 additions & 3 deletions src/windows/wslcsession/WSLCSessionFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ Module Name:
#include "WSLCSessionFactory.h"
#include "WSLCSession.h"
#include "WSLCSessionReference.h"
#include "wslutil.h"
#include "ExecutionContext.h"
Comment thread
beena352 marked this conversation as resolved.
Outdated

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

void wslc::WSLCSessionFactory::SetDestructionCallback(std::function<void()>&& callback)
Expand All @@ -37,6 +36,11 @@ HRESULT wslc::WSLCSessionFactory::CreateSession(
_Out_ IWSLCSessionReference** ServiceRef)
try
{
// Establish a COM execution context so any user-error message set during
// session init (e.g. by ConfigureStorage's THROW_HR_WITH_USER_ERROR) is
// captured and serialized to IErrorInfo before returning to the caller.
Comment thread
beena352 marked this conversation as resolved.
Outdated
wsl::windows::common::COMServiceExecutionContext context;

*Session = nullptr;
*ServiceRef = nullptr;

Expand All @@ -48,7 +52,10 @@ try
session->SetDestructionCallback(std::move(m_destructionCallback));

// Initialize the session with the VM.
RETURN_IF_FAILED(session->Initialize(Settings, Vm, PluginNotifier));
// N.B. THROW_IF_FAILED (not RETURN_IF_FAILED) is required here so WIL's
// error callback invokes CollectError, which consumes the error string
// before ~COMServiceExecutionContext runs.
THROW_IF_FAILED(session->Initialize(Settings, Vm, PluginNotifier));

// Create the service session ref. It extracts metadata and a weak reference from the session.
auto serviceRef = Microsoft::WRL::Make<wslc::WSLCSessionReference>(session.Get());
Expand All @@ -67,6 +74,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
6 changes: 5 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 @@ -52,6 +52,10 @@ class DECLSPEC_UUID("9FCD2067-9FC6-4EFA-9EB0-698169EBF7D3") WSLCSessionFactory

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

// ISupportErrorInfo: enables IErrorInfo marshaling across COM process boundaries so that
// user-facing error messages set via THROW_HR_WITH_USER_ERROR propagate to the caller.
IFACEMETHOD(InterfaceSupportsErrorInfo)(_In_ REFIID riid) override;

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

// Marker-file scenarios use unique temp paths so the test never touches shared default storage.
auto uniqueTempStoragePath = [] {
auto path = std::filesystem::temp_directory_path() /
std::format(L"wslc-test-storage-{}-{}", GetCurrentProcessId(), GetTickCount64());
std::filesystem::create_directories(path);
return path;
};

auto expectMarkerRejection = [&](LPCWSTR name, auto&& populateDir) {
const auto storagePath = uniqueTempStoragePath();
auto cleanup = wil::scope_exit([&]() {
std::error_code ignored;
std::filesystem::remove_all(storagePath, ignored);
});

populateDir(storagePath);

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

// Reject non-empty storage directory without the wslcsession marker.
expectMarkerRejection(
L"storage-not-empty", [](const std::filesystem::path& dir) { std::ofstream{dir / L"userfile.txt"} << "data"; });

// Reject directory whose marker name is occupied by a sub-directory.
expectMarkerRejection(L"marker-is-directory", [](const std::filesystem::path& dir) {
std::filesystem::create_directory(dir / L"wslcsession");
});
}

struct VmInfo
Expand Down
26 changes: 25 additions & 1 deletion test/windows/wslc/e2e/WSLCE2ESessionEnterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,34 @@ 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,
});
}

WSLC_TEST_METHOD(WSLCE2E_SessionEnter_MarkerFileValidation)
{
// The non-empty-rejection case is covered by WSLCTests::CreateSessionValidation against a
// temp directory; reproducing it here would require mutating the shared default storage.
const auto storagePath = GetDefaultStoragePath();
const auto markerPath = storagePath / L"wslcsession";

auto createSessionAndExpectMarker = [&]() {
auto terminateOnExit = wil::scope_exit([&]() { RunWslc(L"system session terminate"); });
RunWslc(L"image ls").Verify({.ExitCode = 0});
VERIFY_IS_TRUE(std::filesystem::exists(markerPath));
};

// Default session creation writes the marker file.
createSessionAndExpectMarker();

// Legacy directory auto-upgrade: removing the marker then re-entering recreates it.
std::error_code ec;
VERIFY_IS_TRUE(std::filesystem::remove(markerPath, ec));
VERIFY_IS_FALSE(std::filesystem::exists(markerPath));
createSessionAndExpectMarker();
}
};
} // namespace WSLCE2ETests