From 666561bfa1a32649ee8ac4b16a832d3814c2379d Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Mon, 22 Nov 2021 18:26:45 +0000 Subject: [PATCH] dll: revert the Delete redesign --- inc/winfsp/winfsp.h | 78 +---------------------------- src/dll/fsop.c | 50 +++++-------------- tst/memfs/memfs.cpp | 95 ++++++++---------------------------- tst/winfsp-tests/exec-test.c | 2 +- 4 files changed, 35 insertions(+), 190 deletions(-) diff --git a/inc/winfsp/winfsp.h b/inc/winfsp/winfsp.h index 78a430d9..44e6c853 100644 --- a/inc/winfsp/winfsp.h +++ b/inc/winfsp/winfsp.h @@ -47,19 +47,6 @@ extern "C" { #endif -/* - * The FILE_DISPOSITION_* definitions appear to be missing from the user mode headers. - */ -#if !defined(FILE_DISPOSITION_DELETE) -#define FILE_DISPOSITION_DO_NOT_DELETE 0x00000000 -#define FILE_DISPOSITION_DELETE 0x00000001 -#define FILE_DISPOSITION_POSIX_SEMANTICS 0x00000002 -/* remaining flags are not needed for user mode file systems but included for completeness */ -#define FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK 0x00000004 -#define FILE_DISPOSITION_ON_CLOSE 0x00000008 -#define FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE 0x00000010 -#endif - /* * The REPARSE_DATA_BUFFER definitions appear to be missing from the user mode headers. */ @@ -361,9 +348,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE /** * Cleanup a file. * - * (NOTE: use of this function with the FspCleanupDelete flag is not recommended; - * use Delete instead.) - * * When CreateFile is used to open or create a file the kernel creates a kernel mode file * object (type FILE_OBJECT) and a handle for it, which it returns to user-mode. The handle may * be duplicated (using DuplicateHandle), but all duplicate handles always refer to the same @@ -418,7 +402,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE * Close * CanDelete * SetDelete - * Delete */ VOID (*Cleanup)(FSP_FILE_SYSTEM *FileSystem, PVOID FileContext, PWSTR FileName, ULONG Flags); @@ -592,8 +575,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE /** * Determine whether a file or directory can be deleted. * - * (NOTE: use of this function is not recommended; use Delete instead.) - * * This function tests whether a file or directory can be safely deleted. This function does * not need to perform access checks, but may performs tasks such as check for empty * directories, etc. @@ -618,7 +599,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE * @see * Cleanup * SetDelete - * Delete */ NTSTATUS (*CanDelete)(FSP_FILE_SYSTEM *FileSystem, PVOID FileContext, PWSTR FileName); @@ -900,8 +880,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE /** * Set the file delete flag. * - * (NOTE: use of this function is not recommended; use Delete instead.) - * * This function sets a flag to indicates whether the FSD file should delete a file * when it is closed. This function does not need to perform access checks, but may * performs tasks such as check for empty directories, etc. @@ -930,7 +908,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE * @see * Cleanup * CanDelete - * Delete */ NTSTATUS (*SetDelete)(FSP_FILE_SYSTEM *FileSystem, PVOID FileContext, PWSTR FileName, BOOLEAN DeleteFile); @@ -1063,59 +1040,8 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE PVOID FileContext, PFILE_FULL_EA_INFORMATION Ea, ULONG EaLength, FSP_FSCTL_FILE_INFO *FileInfo); - /** - * Set the file delete flag or delete a file. - * - * This function replaces CanDelete, SetDelete and uses of Cleanup with the FspCleanupDelete flag - * and is recommended for use in all new code. - * - * Due to the complexity of file deletion in the Windows file system this function is used - * in many scenarios. Its usage is controlled by the Flags parameter: - * - * - * This function gets called in all file deletion scenarios: - * - * - * NOTE: Delete takes precedence over CanDelete, SetDelete and Cleanup with the FspCleanupDelete flag. - * This means that if Delete is defined, CanDelete and SetDelete will never be called and - * Cleanup will never be called with the FspCleanupDelete flag. - * - * @param FileSystem - * The file system on which this request is posted. - * @param FileContext - * The file context of the file or directory to set the delete flag for. - * @param FileName - * The name of the file or directory to set the delete flag for. - * @param Flags - * File disposition flags - * @return - * STATUS_SUCCESS or error code. - * @see - * Cleanup - * CanDelete - * SetDelete - */ - NTSTATUS (*Delete)(FSP_FILE_SYSTEM *FileSystem, - PVOID FileContext, PWSTR FileName, ULONG Flags); + + NTSTATUS (*Obsolete0)(VOID); /* * This ensures that this interface will always contain 64 function pointers. diff --git a/src/dll/fsop.c b/src/dll/fsop.c index 97d90334..eb8f5764 100644 --- a/src/dll/fsop.c +++ b/src/dll/fsop.c @@ -57,9 +57,7 @@ FSP_API NTSTATUS FspFileSystemOpEnter(FSP_FILE_SYSTEM *FileSystem, Request->Req.Cleanup.Delete) || (FspFsctlTransactSetInformationKind == Request->Kind && (10/*FileRenameInformation*/ == Request->Req.SetInformation.FileInformationClass || - 65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass || - (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass && - 3/*DELETE|POSIX_SEMANTICS*/ == (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) || + 65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) || FspFsctlTransactSetVolumeInformationKind == Request->Kind || (FspFsctlTransactFlushBuffersKind == Request->Kind && 0 == Request->Req.FlushBuffers.UserContext && @@ -71,8 +69,7 @@ FSP_API NTSTATUS FspFileSystemOpEnter(FSP_FILE_SYSTEM *FileSystem, if (FspFsctlTransactCreateKind == Request->Kind || (FspFsctlTransactSetInformationKind == Request->Kind && (13/*FileDispositionInformation*/ == Request->Req.SetInformation.FileInformationClass || - (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass && - 3/*DELETE|POSIX_SEMANTICS*/ != (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) || + 64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) || FspFsctlTransactQueryDirectoryKind == Request->Kind || FspFsctlTransactQueryVolumeInformationKind == Request->Kind) { @@ -101,9 +98,7 @@ FSP_API NTSTATUS FspFileSystemOpLeave(FSP_FILE_SYSTEM *FileSystem, Request->Req.Cleanup.Delete) || (FspFsctlTransactSetInformationKind == Request->Kind && (10/*FileRenameInformation*/ == Request->Req.SetInformation.FileInformationClass || - 65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass || - (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass && - 3/*DELETE|POSIX_SEMANTICS*/ == (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) || + 65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) || FspFsctlTransactSetVolumeInformationKind == Request->Kind || (FspFsctlTransactFlushBuffersKind == Request->Kind && 0 == Request->Req.FlushBuffers.UserContext && @@ -115,8 +110,7 @@ FSP_API NTSTATUS FspFileSystemOpLeave(FSP_FILE_SYSTEM *FileSystem, if (FspFsctlTransactCreateKind == Request->Kind || (FspFsctlTransactSetInformationKind == Request->Kind && (13/*FileDispositionInformation*/ == Request->Req.SetInformation.FileInformationClass || - (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass && - 3/*DELETE|POSIX_SEMANTICS*/ != (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) || + 64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) || FspFsctlTransactQueryDirectoryKind == Request->Kind || FspFsctlTransactQueryVolumeInformationKind == Request->Kind) { @@ -991,28 +985,16 @@ FSP_API NTSTATUS FspFileSystemOpOverwrite(FSP_FILE_SYSTEM *FileSystem, FSP_API NTSTATUS FspFileSystemOpCleanup(FSP_FILE_SYSTEM *FileSystem, FSP_FSCTL_TRANSACT_REQ *Request, FSP_FSCTL_TRANSACT_RSP *Response) { - ULONG CleanupFlags = - (0 != Request->Req.Cleanup.Delete ? FspCleanupDelete : 0) | - (0 != Request->Req.Cleanup.SetAllocationSize ? FspCleanupSetAllocationSize : 0) | - (0 != Request->Req.Cleanup.SetArchiveBit ? FspCleanupSetArchiveBit : 0) | - (0 != Request->Req.Cleanup.SetLastAccessTime ? FspCleanupSetLastAccessTime : 0) | - (0 != Request->Req.Cleanup.SetLastWriteTime ? FspCleanupSetLastWriteTime : 0) | - (0 != Request->Req.Cleanup.SetChangeTime ? FspCleanupSetChangeTime : 0); - - if (Request->Req.Cleanup.Delete && 0 != FileSystem->Interface->Delete) - { - FileSystem->Interface->Delete(FileSystem, - (PVOID)ValOfFileContext(Request->Req.Cleanup), - 0 != Request->FileName.Size ? (PWSTR)Request->Buffer : 0, - (ULONG)-1); - CleanupFlags &= ~FspCleanupDelete; - } - if (0 != FileSystem->Interface->Cleanup) FileSystem->Interface->Cleanup(FileSystem, (PVOID)ValOfFileContext(Request->Req.Cleanup), 0 != Request->FileName.Size ? (PWSTR)Request->Buffer : 0, - CleanupFlags); + (0 != Request->Req.Cleanup.Delete ? FspCleanupDelete : 0) | + (0 != Request->Req.Cleanup.SetAllocationSize ? FspCleanupSetAllocationSize : 0) | + (0 != Request->Req.Cleanup.SetArchiveBit ? FspCleanupSetArchiveBit : 0) | + (0 != Request->Req.Cleanup.SetLastAccessTime ? FspCleanupSetLastAccessTime : 0) | + (0 != Request->Req.Cleanup.SetLastWriteTime ? FspCleanupSetLastWriteTime : 0) | + (0 != Request->Req.Cleanup.SetChangeTime ? FspCleanupSetChangeTime : 0)); return STATUS_SUCCESS; } @@ -1171,17 +1153,7 @@ FSP_API NTSTATUS FspFileSystemOpSetInformation(FSP_FILE_SYSTEM *FileSystem, break; } } - if (0 != FileSystem->Interface->Delete) - { - Result = FileSystem->Interface->Delete(FileSystem, - (PVOID)ValOfFileContext(Request->Req.SetInformation), - (PWSTR)Request->Buffer, - Request->Req.SetInformation.Info.DispositionEx.Flags & 3/*DELETE|POSIX_SEMANTICS*/); - } - else if (0 != (2/*POSIX_SEMANTICS*/ & Request->Req.SetInformation.Info.DispositionEx.Flags)) - /* only FSP_FILE_SYSTEM_INTERFACE::Delete can do POSIX semantics; return error if unimplemented */ - Result = STATUS_INVALID_PARAMETER; - else if (0 != FileSystem->Interface->SetDelete) + if (0 != FileSystem->Interface->SetDelete) { Result = FileSystem->Interface->SetDelete(FileSystem, (PVOID)ValOfFileContext(Request->Req.SetInformation), diff --git a/tst/memfs/memfs.cpp b/tst/memfs/memfs.cpp index 07c45fd2..f72534b9 100644 --- a/tst/memfs/memfs.cpp +++ b/tst/memfs/memfs.cpp @@ -88,13 +88,6 @@ FSP_FSCTL_STATIC_ASSERT(MEMFS_MAX_PATH > MAX_PATH, #define MEMFS_REJECT_EARLY_IRP #endif -/* - * Define the MEMFS_DELETE macro to include new Delete support - * (instead of Cleanup/FspCleanupDelete). This is required to - * properly support POSIX unlink/rename. - */ -#define MEMFS_DELETE - /* * Define the DEBUG_BUFFER_CHECK macro on Windows 8 or above. This includes * a check for the Write buffer to ensure that it is read-only. @@ -972,8 +965,6 @@ void SlowioReadDirectoryThread( * FSP_FILE_SYSTEM_INTERFACE */ -static NTSTATUS Delete(FSP_FILE_SYSTEM *FileSystem, - PVOID FileNode0, PWSTR FileName, ULONG Flags); #if defined(MEMFS_REPARSE_POINTS) static NTSTATUS GetReparsePointByName( FSP_FILE_SYSTEM *FileSystem, PVOID Context, @@ -1356,9 +1347,7 @@ static VOID Cleanup(FSP_FILE_SYSTEM *FileSystem, MEMFS_FILE_NODE *MainFileNode = FileNode; #endif -#if !defined(MEMFS_DELETE) assert(0 != Flags); /* FSP_FSCTL_VOLUME_PARAMS::PostCleanupWhenModifiedOnly ensures this */ -#endif if (Flags & FspCleanupSetArchiveBit) { @@ -1387,10 +1376,21 @@ static VOID Cleanup(FSP_FILE_SYSTEM *FileSystem, SetFileSizeInternal(FileSystem, FileNode, AllocationSize, TRUE); } -#if !defined(MEMFS_DELETE) - if (Flags & FspCleanupDelete) - Delete(FileSystem, FileNode0, FileName, -1); + if ((Flags & FspCleanupDelete) && !MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode)) + { +#if defined(MEMFS_NAMED_STREAMS) + MEMFS_FILE_NODE_MAP_ENUM_CONTEXT Context = { FALSE }; + ULONG Index; + + MemfsFileNodeMapEnumerateNamedStreams(Memfs->FileNodeMap, FileNode, + MemfsFileNodeMapEnumerateFn, &Context); + for (Index = 0; Context.Count > Index; Index++) + MemfsFileNodeMapRemove(Memfs->FileNodeMap, Context.FileNodes[Index]); + MemfsFileNodeMapEnumerateFree(&Context); #endif + + MemfsFileNodeMapRemove(Memfs->FileNodeMap, FileNode); + } } static VOID Close(FSP_FILE_SYSTEM *FileSystem, @@ -1651,13 +1651,17 @@ static NTSTATUS SetFileSize(FSP_FILE_SYSTEM *FileSystem, return STATUS_SUCCESS; } -#if !defined(MEMFS_DELETE) static NTSTATUS CanDelete(FSP_FILE_SYSTEM *FileSystem, PVOID FileNode0, PWSTR FileName) { - return Delete(FileSystem, FileNode0, FileName, FILE_DISPOSITION_DELETE); + MEMFS *Memfs = (MEMFS *)FileSystem->UserContext; + MEMFS_FILE_NODE *FileNode = (MEMFS_FILE_NODE *)FileNode0; + + if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode)) + return STATUS_DIRECTORY_NOT_EMPTY; + + return STATUS_SUCCESS; } -#endif static NTSTATUS Rename(FSP_FILE_SYSTEM *FileSystem, PVOID FileNode0, @@ -2227,54 +2231,6 @@ static NTSTATUS SetEa(FSP_FILE_SYSTEM *FileSystem, } #endif -static NTSTATUS Delete(FSP_FILE_SYSTEM *FileSystem, - PVOID FileNode0, PWSTR FileName, ULONG Flags) -{ - MEMFS *Memfs = (MEMFS *)FileSystem->UserContext; - MEMFS_FILE_NODE *FileNode = (MEMFS_FILE_NODE *)FileNode0; - - switch (Flags) - { - case FILE_DISPOSITION_DO_NOT_DELETE: - // set file disposition flag: do not delete file at Cleanup - return STATUS_SUCCESS; - - case FILE_DISPOSITION_DELETE: - // set file disposition flag: delete file at Cleanup - if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode)) - return STATUS_DIRECTORY_NOT_EMPTY; - return STATUS_SUCCESS; - - case FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS: - // delete file now; open handles to file remain valid - /* fallthrough */ - - case -1: - // delete file now; called during Cleanup - if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode)) - return STATUS_DIRECTORY_NOT_EMPTY; - else - { -#if defined(MEMFS_NAMED_STREAMS) - MEMFS_FILE_NODE_MAP_ENUM_CONTEXT Context = { FALSE }; - ULONG Index; - - MemfsFileNodeMapEnumerateNamedStreams(Memfs->FileNodeMap, FileNode, - MemfsFileNodeMapEnumerateFn, &Context); - for (Index = 0; Context.Count > Index; Index++) - MemfsFileNodeMapRemove(Memfs->FileNodeMap, Context.FileNodes[Index]); - MemfsFileNodeMapEnumerateFree(&Context); -#endif - - MemfsFileNodeMapRemove(Memfs->FileNodeMap, FileNode); - return STATUS_SUCCESS; - } - - default: - return STATUS_INVALID_PARAMETER; - } -} - static FSP_FILE_SYSTEM_INTERFACE MemfsInterface = { GetVolumeInfo, @@ -2299,11 +2255,7 @@ static FSP_FILE_SYSTEM_INTERFACE MemfsInterface = GetFileInfo, SetBasicInfo, SetFileSize, -#if !defined(MEMFS_DELETE) CanDelete, -#else - 0, -#endif Rename, GetSecurity, SetSecurity, @@ -2347,11 +2299,6 @@ static FSP_FILE_SYSTEM_INTERFACE MemfsInterface = 0, 0, #endif -#if defined(MEMFS_DELETE) - Delete, -#else - 0, -#endif }; /* diff --git a/tst/winfsp-tests/exec-test.c b/tst/winfsp-tests/exec-test.c index c0c910eb..6eddfcb5 100644 --- a/tst/winfsp-tests/exec-test.c +++ b/tst/winfsp-tests/exec-test.c @@ -209,7 +209,7 @@ static void exec_delete_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout) OPEN_EXISTING, 0, 0); if (INVALID_HANDLE_VALUE != Handle) { - DispositionInfo.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS; + DispositionInfo.Flags = 3/*FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS*/; Success = SetFileInformationByHandle(Handle, 21/*FileDispositionInfoEx*/, &DispositionInfo, sizeof DispositionInfo); ASSERT(!Success);