From 18266a69696349e9641871f55b88ad6e732a8789 Mon Sep 17 00:00:00 2001 From: Ian Butterworth Date: Mon, 29 Jul 2024 17:38:26 -0400 Subject: [PATCH] win,fs: use posix delete semantics, if supported (#4318) Implements posix delete for files and dirs, with fallback to the old method if not supported (e.g. Fat32 or Win8). Fixes: #3839 --- src/win/fs.c | 128 ++++++++++++++++++++++++++++++++--------------- src/win/winapi.h | 13 +++++ test/test-fs.c | 42 +++++++++++++++- test/test-list.h | 2 + 4 files changed, 143 insertions(+), 42 deletions(-) diff --git a/src/win/fs.c b/src/win/fs.c index 19c21571..8414f778 100644 --- a/src/win/fs.c +++ b/src/win/fs.c @@ -46,6 +46,17 @@ #define UV_FS_FREE_PTR 0x0008 #define UV_FS_CLEANEDUP 0x0010 +#ifndef FILE_DISPOSITION_DELETE +#define FILE_DISPOSITION_DELETE 0x0001 +#endif /* FILE_DISPOSITION_DELETE */ + +#ifndef FILE_DISPOSITION_POSIX_SEMANTICS +#define FILE_DISPOSITION_POSIX_SEMANTICS 0x0002 +#endif /* FILE_DISPOSITION_POSIX_SEMANTICS */ + +#ifndef FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE +#define FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE 0x0010 +#endif /* FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE */ #define INIT(subtype) \ do { \ @@ -1061,22 +1072,15 @@ void fs__write(uv_fs_t* req) { } -void fs__rmdir(uv_fs_t* req) { - int result = _wrmdir(req->file.pathw); - if (result == -1) - SET_REQ_WIN32_ERROR(req, _doserrno); - else - SET_REQ_RESULT(req, 0); -} - - -void fs__unlink(uv_fs_t* req) { +static void fs__unlink_rmdir(uv_fs_t* req, BOOL isrmdir) { const WCHAR* pathw = req->file.pathw; HANDLE handle; BY_HANDLE_FILE_INFORMATION info; FILE_DISPOSITION_INFORMATION disposition; + FILE_DISPOSITION_INFORMATION_EX disposition_ex; IO_STATUS_BLOCK iosb; NTSTATUS status; + DWORD error; handle = CreateFileW(pathw, FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES | DELETE, @@ -1097,10 +1101,17 @@ void fs__unlink(uv_fs_t* req) { return; } - if (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { - /* Do not allow deletion of directories, unless it is a symlink. When the - * path refers to a non-symlink directory, report EPERM as mandated by - * POSIX.1. */ + if (isrmdir && !(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { + /* Error if we're in rmdir mode but it is not a dir */ + SET_REQ_UV_ERROR(req, UV_ENOTDIR, ERROR_DIRECTORY); + CloseHandle(handle); + return; + } + + if (!isrmdir && (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { + /* If not explicitly allowed, do not allow deletion of directories, unless + * it is a symlink. When the path refers to a non-symlink directory, report + * EPERM as mandated by POSIX.1. */ /* Check if it is a reparse point. If it's not, it's a normal directory. */ if (!(info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) { @@ -1112,7 +1123,7 @@ void fs__unlink(uv_fs_t* req) { /* Read the reparse point and check if it is a valid symlink. If not, don't * unlink. */ if (fs__readlink_handle(handle, NULL, NULL) < 0) { - DWORD error = GetLastError(); + error = GetLastError(); if (error == ERROR_SYMLINK_NOT_SUPPORTED) error = ERROR_ACCESS_DENIED; SET_REQ_WIN32_ERROR(req, error); @@ -1121,42 +1132,77 @@ void fs__unlink(uv_fs_t* req) { } } - if (info.dwFileAttributes & FILE_ATTRIBUTE_READONLY) { - /* Remove read-only attribute */ - FILE_BASIC_INFORMATION basic = { 0 }; + /* Try posix delete first */ + disposition_ex.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS | + FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE; - basic.FileAttributes = (info.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) | - FILE_ATTRIBUTE_ARCHIVE; - - status = pNtSetInformationFile(handle, - &iosb, - &basic, - sizeof basic, - FileBasicInformation); - if (!NT_SUCCESS(status)) { - SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status)); - CloseHandle(handle); - return; - } - } - - /* Try to set the delete flag. */ - disposition.DeleteFile = TRUE; status = pNtSetInformationFile(handle, &iosb, - &disposition, - sizeof disposition, - FileDispositionInformation); + &disposition_ex, + sizeof disposition_ex, + FileDispositionInformationEx); if (NT_SUCCESS(status)) { SET_REQ_SUCCESS(req); } else { - SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status)); + /* If status == STATUS_CANNOT_DELETE here, given we set + * FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE, STATUS_CANNOT_DELETE can only mean + * that there is an existing mapped view to the file, preventing delete. + * STATUS_CANNOT_DELETE maps to UV_EACCES so it's not specifically worth handling */ + error = pRtlNtStatusToDosError(status); + if (error == ERROR_NOT_SUPPORTED /* filesystem does not support posix deletion */ || + error == ERROR_INVALID_PARAMETER /* pre Windows 10 error */ || + error == ERROR_INVALID_FUNCTION /* pre Windows 10 1607 error */) { + /* posix delete not supported so try fallback */ + if (info.dwFileAttributes & FILE_ATTRIBUTE_READONLY) { + /* Remove read-only attribute */ + FILE_BASIC_INFORMATION basic = { 0 }; + + basic.FileAttributes = (info.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY) | + FILE_ATTRIBUTE_ARCHIVE; + + status = pNtSetInformationFile(handle, + &iosb, + &basic, + sizeof basic, + FileBasicInformation); + if (!NT_SUCCESS(status)) { + SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status)); + CloseHandle(handle); + return; + } + } + + /* Try to set the delete flag. */ + disposition.DeleteFile = TRUE; + status = pNtSetInformationFile(handle, + &iosb, + &disposition, + sizeof disposition, + FileDispositionInformation); + if (NT_SUCCESS(status)) { + SET_REQ_SUCCESS(req); + } else { + SET_REQ_WIN32_ERROR(req, pRtlNtStatusToDosError(status)); + } + } else { + SET_REQ_WIN32_ERROR(req, error); + } } CloseHandle(handle); } +static void fs__rmdir(uv_fs_t* req) { + fs__unlink_rmdir(req, /*isrmdir*/1); +} + + +static void fs__unlink(uv_fs_t* req) { + fs__unlink_rmdir(req, /*isrmdir*/0); +} + + void fs__mkdir(uv_fs_t* req) { /* TODO: use req->mode. */ if (CreateDirectoryW(req->file.pathw, NULL)) { @@ -1182,7 +1228,7 @@ void fs__mktemp(uv_fs_t* req, uv__fs_mktemp_func func) { size_t len; uint64_t v; char* path; - + path = (char*)req->path; len = wcslen(req->file.pathw); ep = req->file.pathw + len; @@ -1655,7 +1701,7 @@ INLINE static int fs__stat_handle(HANDLE handle, uv_stat_t* statbuf, statbuf->st_mode |= (_S_IREAD | _S_IWRITE) | ((_S_IREAD | _S_IWRITE) >> 3) | ((_S_IREAD | _S_IWRITE) >> 6); statbuf->st_nlink = 1; - statbuf->st_blksize = 4096; + statbuf->st_blksize = 4096; statbuf->st_rdev = FILE_DEVICE_NULL << 16; return 0; } diff --git a/src/win/winapi.h b/src/win/winapi.h index d380bda4..d3449c18 100644 --- a/src/win/winapi.h +++ b/src/win/winapi.h @@ -4224,6 +4224,15 @@ typedef enum _FILE_INFORMATION_CLASS { FileNumaNodeInformation, FileStandardLinkInformation, FileRemoteProtocolInformation, + FileRenameInformationBypassAccessCheck, + FileLinkInformationBypassAccessCheck, + FileVolumeNameInformation, + FileIdInformation, + FileIdExtdDirectoryInformation, + FileReplaceCompletionInformation, + FileHardLinkFullIdInformation, + FileIdExtdBothDirectoryInformation, + FileDispositionInformationEx, /* based on https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ne-wdm-_file_information_class */ FileMaximumInformation } FILE_INFORMATION_CLASS, *PFILE_INFORMATION_CLASS; @@ -4323,6 +4332,10 @@ typedef struct _FILE_DISPOSITION_INFORMATION { BOOLEAN DeleteFile; } FILE_DISPOSITION_INFORMATION, *PFILE_DISPOSITION_INFORMATION; +typedef struct _FILE_DISPOSITION_INFORMATION_EX { + DWORD Flags; +} FILE_DISPOSITION_INFORMATION_EX, *PFILE_DISPOSITION_INFORMATION_EX; + typedef struct _FILE_PIPE_LOCAL_INFORMATION { ULONG NamedPipeType; ULONG NamedPipeConfiguration; diff --git a/test/test-fs.c b/test/test-fs.c index fe78117b..ccecb189 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -104,6 +104,7 @@ static uv_loop_t* loop; static uv_fs_t open_req1; static uv_fs_t open_req2; +static uv_fs_t open_req_noclose; static uv_fs_t read_req; static uv_fs_t write_req; static uv_fs_t unlink_req; @@ -304,7 +305,7 @@ static void chown_root_cb(uv_fs_t* req) { ASSERT_EQ(req->result, UV_EINVAL); # elif defined(__PASE__) /* On IBMi PASE, there is no root user. uid 0 is user qsecofr. - * User may grant qsecofr's privileges, including changing + * User may grant qsecofr's privileges, including changing * the file's ownership to uid 0. */ ASSERT(req->result == 0 || req->result == UV_EPERM); @@ -1067,6 +1068,45 @@ TEST_IMPL(fs_file_sync) { return 0; } +TEST_IMPL(fs_posix_delete) { + int r; + + /* Setup. */ + unlink("test_dir/file"); + rmdir("test_dir"); + + r = uv_fs_mkdir(NULL, &mkdir_req, "test_dir", 0755, NULL); + ASSERT_OK(r); + + r = uv_fs_open(NULL, &open_req_noclose, "test_dir/file", UV_FS_O_WRONLY | UV_FS_O_CREAT, S_IWUSR | S_IRUSR, NULL); + ASSERT_GE(r, 0); + uv_fs_req_cleanup(&open_req_noclose); + + /* should not be possible to delete the non-empty dir */ + r = uv_fs_rmdir(NULL, &rmdir_req, "test_dir", NULL); + ASSERT_EQ(r, UV_ENOTEMPTY); + ASSERT_EQ(r, rmdir_req.result); + uv_fs_req_cleanup(&rmdir_req); + + r = uv_fs_unlink(NULL, &unlink_req, "test_dir/file", NULL); + ASSERT_OK(r); + ASSERT_OK(unlink_req.result); + uv_fs_req_cleanup(&unlink_req); + + /* delete the dir while the file is still open, which should succeed on posix */ + r = uv_fs_rmdir(NULL, &rmdir_req, "test_dir", NULL); + ASSERT_OK(r); + ASSERT_OK(rmdir_req.result); + uv_fs_req_cleanup(&rmdir_req); + + /* Cleanup */ + r = uv_fs_close(NULL, &close_req, open_req_noclose.result, NULL); + ASSERT_OK(r); + uv_fs_req_cleanup(&close_req); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} static void fs_file_write_null_buffer(int add_flags) { int r; diff --git a/test/test-list.h b/test/test-list.h index d600aef2..68878cf8 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -354,6 +354,7 @@ TEST_DECLARE (fs_file_nametoolong) TEST_DECLARE (fs_file_loop) TEST_DECLARE (fs_file_async) TEST_DECLARE (fs_file_sync) +TEST_DECLARE (fs_posix_delete) TEST_DECLARE (fs_file_write_null_buffer) TEST_DECLARE (fs_async_dir) TEST_DECLARE (fs_async_sendfile) @@ -1062,6 +1063,7 @@ TASK_LIST_START TEST_ENTRY (fs_file_loop) TEST_ENTRY (fs_file_async) TEST_ENTRY (fs_file_sync) + TEST_ENTRY (fs_posix_delete) TEST_ENTRY (fs_file_write_null_buffer) TEST_ENTRY (fs_async_dir) TEST_ENTRY (fs_async_sendfile)