sys: FspFileNodeRename: correctly handle rename of closed by referenced descendant files

src/sys/file.c

@@ -806,23 +806,94 @@ VOID FspFileNodeRename(FSP_FILE_NODE *FileNode, PUNICODE_STRING NewFileName)
     PAGED_CODE();
 
     PDEVICE_OBJECT FsvolDeviceObject = FileNode->FsvolDeviceObject;
+    FSP_FILE_NODE *DescendantFileNode;
+    FSP_FILE_NODE *DescendantFileNodeArray[16], **DescendantFileNodes;
+    ULONG DescendantFileNodeCount, DescendantFileNodeIndex;
+    FSP_DEVICE_CONTEXT_BY_NAME_TABLE_RESTART_KEY RestartKey;
     BOOLEAN Deleted, Inserted;
+    USHORT FileNameLength;
+    PWSTR ExternalFileName;
 
     FspFsvolDeviceLockContextTable(FsvolDeviceObject);
 
-    FspFsvolDeviceDeleteContextByName(FsvolDeviceObject, &FileNode->FileName, &Deleted);
+    DescendantFileNodes = DescendantFileNodeArray;
-    ASSERT(Deleted);
+    DescendantFileNodes[0] = FileNode;
+    DescendantFileNodeCount = 1;
+    memset(&RestartKey, 0, sizeof RestartKey);
+    for (;;)
+    {
+        DescendantFileNode = FspFsvolDeviceEnumerateContextByName(FsvolDeviceObject,
+            &FileNode->FileName, TRUE, &RestartKey);
+        if (0 == DescendantFileNode)
+            break;
 
-    if (0 != FileNode->ExternalFileName)
+        if (ARRAYSIZE(DescendantFileNodeArray) > DescendantFileNodeCount)
-        FspFree(FileNode->ExternalFileName);
+            DescendantFileNodes[DescendantFileNodeCount] = DescendantFileNode;
-    FileNode->FileName = *NewFileName;
+        DescendantFileNodeCount++;
-    FileNode->ExternalFileName = NewFileName->Buffer;
+    }
 
-    FspFsvolDeviceInsertContextByName(FsvolDeviceObject, &FileNode->FileName, FileNode,
+    if (ARRAYSIZE(DescendantFileNodeArray) < DescendantFileNodeCount)
-        &FileNode->ContextByNameElementStorage, &Inserted);
+    {
-    ASSERT(Inserted);
+        DescendantFileNodes = FspAllocMustSucceed(DescendantFileNodeCount * sizeof(FSP_FILE_NODE *));
+        DescendantFileNodes[0] = FileNode;
+        DescendantFileNodeIndex = 1;
+        memset(&RestartKey, 0, sizeof RestartKey);
+        for (;;)
+        {
+            DescendantFileNode = FspFsvolDeviceEnumerateContextByName(FsvolDeviceObject,
+                &FileNode->FileName, TRUE, &RestartKey);
+            if (0 == DescendantFileNode)
+                break;
+
+            DescendantFileNodes[DescendantFileNodeIndex] = DescendantFileNode;
+            DescendantFileNodeIndex++;
+            ASSERT(DescendantFileNodeCount >= DescendantFileNodeIndex);
+        }
+
+        ASSERT(DescendantFileNodeCount == DescendantFileNodeIndex);
+    }
+
+    FileNameLength = FileNode->FileName.Length;
+    for (
+        DescendantFileNodeIndex = 0;
+        DescendantFileNodeCount > DescendantFileNodeIndex;
+        DescendantFileNodeIndex++)
+    {
+        DescendantFileNode = DescendantFileNodes[DescendantFileNodeIndex];
+        ASSERT(DescendantFileNode->FileName.Length >= FileNameLength);
+
+        FspFsvolDeviceDeleteContextByName(FsvolDeviceObject, &DescendantFileNode->FileName, &Deleted);
+        ASSERT(Deleted);
+
+        ExternalFileName = DescendantFileNode->ExternalFileName;
+
+        DescendantFileNode->FileName.MaximumLength =
+            DescendantFileNode->FileName.Length - FileNameLength + NewFileName->Length;
+        DescendantFileNode->ExternalFileName = FspAllocMustSucceed(
+            DescendantFileNode->FileName.MaximumLength);
+
+        RtlCopyMemory((PUINT8)DescendantFileNode->ExternalFileName + NewFileName->Length,
+            (PUINT8)DescendantFileNode->FileName.Buffer + FileNameLength,
+            DescendantFileNode->FileName.Length - FileNameLength);
+        RtlCopyMemory(DescendantFileNode->ExternalFileName,
+            NewFileName->Buffer,
+            NewFileName->Length);
+
+        DescendantFileNode->FileName.Length = DescendantFileNode->FileName.MaximumLength;
+        DescendantFileNode->FileName.Buffer = DescendantFileNode->ExternalFileName;
+
+        if (0 != ExternalFileName)
+            FspFree(ExternalFileName);
+
+        FspFsvolDeviceInsertContextByName(FsvolDeviceObject, &DescendantFileNode->FileName, DescendantFileNode,
+            &DescendantFileNode->ContextByNameElementStorage, &Inserted);
+        ASSERT(Inserted);
+    }
 
     FspFsvolDeviceUnlockContextTable(FsvolDeviceObject);
+
+    if (DescendantFileNodeArray != DescendantFileNodes)
+        FspFree(DescendantFileNodes);
 }
 
 BOOLEAN FspFileNodeHasOpenHandles(PDEVICE_OBJECT FsvolDeviceObject,

src/sys/fileinfo.c

@@ -1229,9 +1229,8 @@ static NTSTATUS FspFsvolSetRenameInformationSuccess(
 
     NewFileName.Length = NewFileName.MaximumLength =
         Request->Req.SetInformation.Info.Rename.NewFileName.Size - sizeof(WCHAR);
-    NewFileName.Buffer = FspAllocMustSucceed(NewFileName.Length);
+    NewFileName.Buffer = (PVOID)
-    RtlCopyMemory(NewFileName.Buffer, Request->Buffer + Request->FileName.Size, NewFileName.Length);
+        (Request->Buffer + Request->Req.SetInformation.Info.Rename.NewFileName.Offset);
-
     FspFileNodeRename(FileNode, &NewFileName);
 
     /* fastfat has some really arcane rules on rename notifications; simplify! */

tst/winfsp-tests/info-test.c

@@ -462,6 +462,120 @@ void rename_test(void)
     }
 }
 
+static void rename_flipflop_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout, ULONG NumMappings)
+{
+    void *memfs = memfs_start_ex(Flags, FileInfoTimeout);
+
+    HANDLE Handle, Mappings[80];
+    BOOL Success;
+    WCHAR FilePath[MAX_PATH];
+    WCHAR FilePath2[MAX_PATH];
+    SYSTEM_INFO SystemInfo;
+
+    ASSERT(ARRAYSIZE(Mappings) >= NumMappings);
+
+    GetSystemInfo(&SystemInfo);
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+    Success = CreateDirectoryW(FilePath, 0);
+    ASSERT(Success);
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\subdir",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+    Success = CreateDirectoryW(FilePath, 0);
+    ASSERT(Success);
+
+    for (ULONG j = 1; NumMappings >= j; j++)
+    {
+        if (NumMappings / 2 >= j)
+            StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\%.*s",
+                Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs),
+                j, L"01234567890123456789012345678901234567890123456789012345678901234567890123456789");
+        else
+            StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\subdir\\%.*s",
+                Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs),
+                j, L"01234567890123456789012345678901234567890123456789012345678901234567890123456789");
+        Handle = CreateFileW(FilePath, GENERIC_ALL, 0, 0, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
+        ASSERT(INVALID_HANDLE_VALUE != Handle);
+        Mappings[j - 1] = CreateFileMappingW(Handle, 0, PAGE_READWRITE,
+            0, SystemInfo.dwAllocationGranularity, 0);
+        ASSERT(0 != Mappings[j - 1]);
+        Success = CloseHandle(Handle);
+        ASSERT(Success);
+    }
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+
+    StringCbPrintfW(FilePath2, sizeof FilePath2, L"%s%s\\longlonglonglonglonglonglonglong",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+
+    for (ULONG i = 0; 10 > i; i++)
+    {
+        Success = MoveFileExW(FilePath, FilePath2, 0);
+        ASSERT(Success);
+        Success = MoveFileExW(FilePath2, FilePath, 0);
+        ASSERT(Success);
+    }
+
+    for (ULONG j = 1; NumMappings >= j; j++)
+    {
+        Success = CloseHandle(Mappings[j - 1]);
+        ASSERT(Success);
+    }
+
+    for (ULONG j = 1; NumMappings >= j; j++)
+    {
+        if (NumMappings / 2 >= j)
+            StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\%.*s",
+                Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs),
+                j, L"01234567890123456789012345678901234567890123456789012345678901234567890123456789");
+        else
+            StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\subdir\\%.*s",
+                Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs),
+                j, L"01234567890123456789012345678901234567890123456789012345678901234567890123456789");
+        Success = DeleteFileW(FilePath);
+        ASSERT(Success);
+    }
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short\\subdir",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+    Success = RemoveDirectoryW(FilePath);
+    ASSERT(Success);
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\short",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+    Success = RemoveDirectoryW(FilePath);
+    ASSERT(Success);
+
+    memfs_stop(memfs);
+}
+
+void rename_flipflop_test(void)
+{
+    if (NtfsTests)
+    {
+        WCHAR DirBuf[MAX_PATH] = L"\\\\?\\";
+        GetCurrentDirectoryW(MAX_PATH - 4, DirBuf + 4);
+        rename_flipflop_dotest(-1, DirBuf, 0, 10);
+    }
+    if (WinFspDiskTests)
+    {
+        rename_flipflop_dotest(MemfsDisk, 0, 0, 10);
+        rename_flipflop_dotest(MemfsDisk, 0, 1000, 10);
+        rename_flipflop_dotest(MemfsDisk, 0, 0, 40);
+        rename_flipflop_dotest(MemfsDisk, 0, 1000, 40);
+    }
+    if (WinFspNetTests)
+    {
+        rename_flipflop_dotest(MemfsNet, L"\\\\memfs\\share", 0, 10);
+        rename_flipflop_dotest(MemfsNet, L"\\\\memfs\\share", 1000, 10);
+        rename_flipflop_dotest(MemfsNet, L"\\\\memfs\\share", 0, 40);
+        rename_flipflop_dotest(MemfsNet, L"\\\\memfs\\share", 1000, 40);
+    }
+}
+
 void getvolinfo_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout)
 {
     void *memfs = memfs_start_ex(Flags, FileInfoTimeout);
@@ -639,6 +753,7 @@ void info_tests(void)
     TEST(delete_test);
     TEST(delete_access_test);
     TEST(rename_test);
+    TEST(rename_flipflop_test);
     TEST(getvolinfo_test);
     TEST(setvolinfo_test);
 }