diff --git a/src/windows/inc/WslPluginApi.h b/src/windows/inc/WslPluginApi.h index c969c080..bcdcdb8f 100644 --- a/src/windows/inc/WslPluginApi.h +++ b/src/windows/inc/WslPluginApi.h @@ -26,9 +26,6 @@ extern "C" { #define WSLPLUGINAPI_ENTRYPOINTV1 WSLPluginAPIV1_EntryPoint #define WSL_E_PLUGIN_REQUIRES_UPDATE MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, 0x032A) -// Maximum size for mount points returned by WSLCPluginAPI_MountFolder. This includes the null terminator. -#define WSLC_MOUNTPOINT_LENGTH 256 - #define WSL_PLUGIN_REQUIRE_VERSION(_Major, _Minor, _Revision, Api) \ if (Api->Version.Major < (_Major) || (Api->Version.Major == (_Major) && Api->Version.Minor < (_Minor)) || \ (Api->Version.Major == (_Major) && Api->Version.Minor == (_Minor) && Api->Version.Revision < (_Revision))) \ @@ -148,9 +145,8 @@ typedef HRESULT (*WSLPluginAPI_ImageDeleted)(const struct WSLCSessionInformation // WSLC plugin API calls. // -// Mount a Windows folder into the WSLC session VM. The mount path is returned via 'Mountpoint'. -// 'Mountpoint' must point to a buffer of at least WSLC_MOUNTPOINT_LENGTH chars, including the null terminator. -typedef HRESULT (*WSLCPluginAPI_MountFolder)(WSLCSessionId Session, LPCWSTR WindowsPath, BOOL ReadOnly, LPCWSTR Name, LPSTR Mountpoint); +// Mount a Windows folder into the WSLC session VM at the given 'Mountpoint' path. If the 'Mountpoint' doesn't exist, it will be created. +typedef HRESULT (*WSLCPluginAPI_MountFolder)(WSLCSessionId Session, LPCWSTR WindowsPath, LPCSTR Mountpoint, BOOL ReadOnly); // Unmount a folder previously mounted via WSLCPluginAPI_MountFolder. typedef HRESULT (*WSLCPluginAPI_UnmountFolder)(WSLCSessionId Session, LPCSTR Mountpoint); diff --git a/src/windows/service/exe/PluginManager.cpp b/src/windows/service/exe/PluginManager.cpp index 38c1bfbd..9e7f0349 100644 --- a/src/windows/service/exe/PluginManager.cpp +++ b/src/windows/service/exe/PluginManager.cpp @@ -120,42 +120,23 @@ wil::com_ptr ResolveWslcSession(WSLCSessionId Session) extern "C" { -HRESULT WSLCMountFolder(WSLCSessionId Session, LPCWSTR WindowsPath, BOOL ReadOnly, LPCWSTR Name, LPSTR Mountpoint) +HRESULT WSLCMountFolder(WSLCSessionId Session, LPCWSTR WindowsPath, LPCSTR Mountpoint, BOOL ReadOnly) try { - RETURN_HR_IF(E_POINTER, WindowsPath == nullptr || Name == nullptr || Mountpoint == nullptr); - auto nameLength = wcslen(Name); - - RETURN_HR_IF_MSG( - E_INVALIDARG, - nameLength == 0 || - !std::ranges::all_of(Name, Name + nameLength, [&](auto c) { return c == '-' || c == '_' || iswalnum(c); }), - "Invalid mount name: %ls", - Name); + // TODO: Once plugins are out of proc, add logic to validate that the mountpoint isn't in use by another plugin. + RETURN_HR_IF(E_POINTER, WindowsPath == nullptr || Mountpoint == nullptr); auto session = ResolveWslcSession(Session); - - // Mount the folder under /mnt/wsl-plugin/. Convert Name to UTF-8 for the Linux path. - const auto linuxPath = std::format("/mnt/wsl-plugin/{}", Name); - - THROW_HR_IF_MSG(E_INVALIDARG, linuxPath.length() >= WSLC_MOUNTPOINT_LENGTH, "Mountpoint too long: %hs", linuxPath.c_str()); - - auto result = session->MountWindowsFolder(WindowsPath, linuxPath.c_str(), ReadOnly); + auto result = session->MountWindowsFolder(WindowsPath, Mountpoint, ReadOnly); WSL_LOG( "WslcPluginMountFolderCall", TraceLoggingValue(Session, "SessionId"), TraceLoggingValue(WindowsPath, "WindowsPath"), - TraceLoggingValue(linuxPath.c_str(), "LinuxPath"), + TraceLoggingValue(Mountpoint, "Mountpoint"), TraceLoggingValue(ReadOnly, "ReadOnly"), - TraceLoggingValue(Name, "Name"), TraceLoggingValue(result, "Result")); - if (SUCCEEDED(result)) - { - THROW_HR_IF(E_UNEXPECTED, strcpy_s(Mountpoint, WSLC_MOUNTPOINT_LENGTH, linuxPath.c_str()) != 0); - } - return result; } CATCH_RETURN(); @@ -163,6 +144,7 @@ CATCH_RETURN(); HRESULT WSLCUnmountFolder(WSLCSessionId Session, LPCSTR Mountpoint) try { + // TODO: Once plugins are out of proc, add logic to validate that the mountpoint is actually owned by the plugin. RETURN_HR_IF(E_POINTER, Mountpoint == nullptr); auto session = ResolveWslcSession(Session); diff --git a/src/windows/wslcsession/WSLCVirtualMachine.cpp b/src/windows/wslcsession/WSLCVirtualMachine.cpp index ba7748de..6d19750a 100644 --- a/src/windows/wslcsession/WSLCVirtualMachine.cpp +++ b/src/windows/wslcsession/WSLCVirtualMachine.cpp @@ -1008,6 +1008,8 @@ try THROW_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), !std::filesystem::is_directory(path), "Path is not a directory: '%ls'", WindowsPath); + THROW_HR_IF_MSG(E_INVALIDARG, LinuxPath[0] != '/', "Mountpoint is not absolute: '%hs'", LinuxPath); + const bool readOnly = WI_IsFlagSet(Flags, WSLCMountFlagsReadOnly); auto normalizedPath = std::filesystem::weakly_canonical(path).wstring(); GUID shareGuid{}; diff --git a/test/windows/PluginTests.cpp b/test/windows/PluginTests.cpp index c9b11288..61b28a74 100644 --- a/test/windows/PluginTests.cpp +++ b/test/windows/PluginTests.cpp @@ -664,8 +664,7 @@ class PluginTests WSLC RO folder mounted at: /mnt/wsl-plugin/plugin-ro-test Command: 'echo fail > /mnt/wsl-plugin/plugin-ro-test/should-not-exist.txt', status=1, stdout: , stderr: * WSLCMountFolder(nonexistent): {} - WSLCMountFolder(../escape): {} - WSLCMountFolder(): {} + WSLCMountFolder(relative): {} Test completed WSLC Container started, session=*, id=*, name=wslc-plugin-container, image=debian:latest, state=* WSLC Container stopping, session=*, id=* @@ -675,7 +674,6 @@ class PluginTests E_INVALIDARG, HRESULT_FROM_WIN32(ERROR_INVALID_STATE), HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND), - E_INVALIDARG, E_INVALIDARG); ValidateLogFile(ExpectedOutput.c_str()); diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index def5ff13..d7ec4942 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -3233,6 +3233,8 @@ class WSLCTests { VERIFY_ARE_EQUAL(session->MountWindowsFolder(L"relative-path", "/win-path", true), E_INVALIDARG); VERIFY_ARE_EQUAL(session->MountWindowsFolder(L"C:\\does-not-exist", "/win-path", true), HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND)); + VERIFY_ARE_EQUAL(session->MountWindowsFolder(testFolder.c_str(), "relative-mountpoint", true), E_INVALIDARG); + VERIFY_ARE_EQUAL(session->MountWindowsFolder(testFolder.c_str(), "", true), E_INVALIDARG); VERIFY_ARE_EQUAL(session->UnmountWindowsFolder("/not-mounted"), HRESULT_FROM_WIN32(ERROR_NOT_FOUND)); VERIFY_ARE_EQUAL(session->UnmountWindowsFolder("/proc"), HRESULT_FROM_WIN32(ERROR_NOT_FOUND)); diff --git a/test/windows/testplugin/Plugin.cpp b/test/windows/testplugin/Plugin.cpp index af5fc5f9..17087e6e 100644 --- a/test/windows/testplugin/Plugin.cpp +++ b/test/windows/testplugin/Plugin.cpp @@ -441,6 +441,8 @@ try const auto testFolder = L"C:\\"; constexpr auto testFileName = L"plugin-test.txt"; + constexpr auto rwMountpoint = "/mnt/wsl-plugin/plugin-rw-test"; + constexpr auto roMountpoint = "/mnt/wsl-plugin/plugin-ro-test"; // Validate rw mounts. { @@ -453,8 +455,7 @@ try } // Mount read-write and verify the file can be read from Linux. - char rwMountpoint[WSLC_MOUNTPOINT_LENGTH] = {}; - THROW_IF_FAILED(g_api->WSLCMountFolder(Session->SessionId, testFolder, false, L"plugin-rw-test", rwMountpoint)); + THROW_IF_FAILED(g_api->WSLCMountFolder(Session->SessionId, testFolder, rwMountpoint, false)); g_logfile << "WSLC RW folder mounted at: " << rwMountpoint << std::endl; @@ -466,8 +467,7 @@ try // Validate ro mounts. { - char roMountpoint[WSLC_MOUNTPOINT_LENGTH] = {}; - THROW_IF_FAILED(g_api->WSLCMountFolder(Session->SessionId, L"C:\\", TRUE, L"plugin-ro-test", roMountpoint)); + THROW_IF_FAILED(g_api->WSLCMountFolder(Session->SessionId, L"C:\\", roMountpoint, TRUE)); g_logfile << "WSLC RO folder mounted at: " << roMountpoint << std::endl; @@ -479,24 +479,12 @@ try } // Validate that trying to mount a folder that doesn't exist fails with the expected error code. - { - char mountpoint[WSLC_MOUNTPOINT_LENGTH] = {}; - g_logfile << "WSLCMountFolder(nonexistent): " - << g_api->WSLCMountFolder(Session->SessionId, L"C:\\nonexistent", TRUE, L"plugin-ro-test", mountpoint) << std::endl; - } + g_logfile << "WSLCMountFolder(nonexistent): " << g_api->WSLCMountFolder(Session->SessionId, L"C:\\nonexistent", roMountpoint, TRUE) + << std::endl; - // Validate that trying to escape the /mnt folder fails. - { - char mountpoint[WSLC_MOUNTPOINT_LENGTH] = {}; - g_logfile << "WSLCMountFolder(../escape): " << g_api->WSLCMountFolder(Session->SessionId, L"C:\\", TRUE, L"../escape", mountpoint) - << std::endl; - } - - // Validate that empty names are rejected. - { - char mountpoint[WSLC_MOUNTPOINT_LENGTH] = {}; - g_logfile << "WSLCMountFolder(): " << g_api->WSLCMountFolder(Session->SessionId, L"C:\\", TRUE, L"", mountpoint) << std::endl; - } + // Validate that non-absolute mountpoints are rejected. + g_logfile << "WSLCMountFolder(relative): " << g_api->WSLCMountFolder(Session->SessionId, L"C:\\", "relative-mountpoint", TRUE) + << std::endl; g_logfile << "Test completed" << std::endl; }