[PATCH v13 0/4] MR6855: server: Fix renaming file to a hardlink of itself.

[Gabriel Ivăncescu (＠insn)]

--
  v13: server: Handle hardlinks and casefolding when renaming the same file.
       server: Handle renames to destinations containing trailing slashes.
       kernel32/tests: Test renaming a file into a hardlink of itself.
       kernel32/tests: Use FindClose instead of CloseHandle when closing

https://gitlab.winehq.org/wine/wine/-/merge_requests/6855

[PATCH v13 1/4] kernel32/tests: Use FindClose instead of CloseHandle when closing FindFirstFile handle.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/kernel32/tests/file.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 38c68297097..126c28ca440 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -1994,9 +1994,8 @@ static void test_MoveFileA(void)
     char tempdir[MAX_PATH];
     char source[MAX_PATH], dest[MAX_PATH];
     static const char prefix[] = "pfx";
+    HANDLE hfile, hfind, hmapfile;
     WIN32_FIND_DATAA find_data;
-    HANDLE hfile;
-    HANDLE hmapfile;
     DWORD ret;
     BOOL retok;

@@ -2076,14 +2075,14 @@ static void test_MoveFileA(void)
     ret = MoveFileA(source, tempdir);
     ok(ret, "MoveFileA: failed, error %ld\n", GetLastError());

-    hfile = FindFirstFileA(tempdir, &find_data);
-    ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
-    if (hfile != INVALID_HANDLE_VALUE)
+    hfind = FindFirstFileA(tempdir, &find_data);
+    ok(hfind != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
+    if (hfind != INVALID_HANDLE_VALUE)
     {
         todo_wine ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName),
            "MoveFile failed to change casing on same file: got %s\n", find_data.cFileName);
     }
-    CloseHandle(hfile);
+    FindClose(hfind);

/* test renaming another file "Remove Be" to "Remove Me", which replaces the existing "Remove me" */
     tempdir[lstrlenA(tempdir) - 2] = 'B';
@@ -2100,14 +2099,14 @@ static void test_MoveFileA(void)

tempdir[lstrlenA(tempdir) - 2] = 'm';

-    hfile = FindFirstFileA(tempdir, &find_data);
-    ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
-    if (hfile != INVALID_HANDLE_VALUE)
+    hfind = FindFirstFileA(tempdir, &find_data);
+    ok(hfind != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
+    if (hfind != INVALID_HANDLE_VALUE)
     {
         ok(!lstrcmpA(strrchr(source, '\') + 1, find_data.cFileName),
            "MoveFile failed to change casing on existing target file: got %s\n", find_data.cFileName);
     }
-    CloseHandle(hfile);
+    FindClose(hfind);

ret = DeleteFileA(tempdir);
     ok(ret, "DeleteFileA: error %ld\n", GetLastError());
@@ -2121,14 +2120,14 @@ static void test_MoveFileA(void)
     ret = MoveFileA(source, tempdir);
     ok(ret, "MoveFileA: failed, error %ld\n", GetLastError());

-    hfile = FindFirstFileA(tempdir, &find_data);
-    ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
-    if (hfile != INVALID_HANDLE_VALUE)
+    hfind = FindFirstFileA(tempdir, &find_data);
+    ok(hfind != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
+    if (hfind != INVALID_HANDLE_VALUE)
     {
         todo_wine ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName),
            "MoveFile failed to change casing on same directory: got %s\n", find_data.cFileName);
     }
-    CloseHandle(hfile);
+    FindClose(hfind);

lstrcpyA(source, dest);
     lstrcpyA(dest, tempdir);
@@ -2143,12 +2142,11 @@ static void test_MoveFileA(void)
     {
         WIN32_FIND_DATAA fd;
         char temppath[MAX_PATH];
-        HANDLE hFind;

lstrcpyA(temppath, tempdir);
         lstrcatA(temppath, "\*.*");
-        hFind = FindFirstFileA(temppath, &fd);
-        if (INVALID_HANDLE_VALUE != hFind)
+        hfind = FindFirstFileA(temppath, &fd);
+        if (INVALID_HANDLE_VALUE != hfind)
         {
           LPSTR lpName;
           do
@@ -2158,8 +2156,8 @@ static void test_MoveFileA(void)
               lpName = fd.cFileName;
             ok(IsDotDir(lpName), "MoveFileA: wildcards file created!\n");
           }
-          while (FindNextFileA(hFind, &fd));
-          FindClose(hFind);
+          while (FindNextFileA(hfind, &fd));
+          FindClose(hfind);
         }
     }
     ret = DeleteFileA(source);
@@ -2893,7 +2891,7 @@ static void test_FindFirstFileA(void)
     if (handle == INVALID_HANDLE_VALUE)
         ok( err == ERROR_PATH_NOT_FOUND, "Bad Error number %d\n", err );
     else
-        CloseHandle( handle );
+        FindClose( handle );

/* try FindFirstFileA on "c:\foo\nul\bar" */
     SetLastError( 0xdeadbeaf );

[PATCH v13 2/4] kernel32/tests: Test renaming a file into a hardlink of itself.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/kernel32/tests/file.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 126c28ca440..e81fa0a4e20 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -2160,10 +2160,34 @@ static void test_MoveFileA(void)
           FindClose(hfind);
         }
     }
-    ret = DeleteFileA(source);
-    ok(ret, "DeleteFileA: error %ld\n", GetLastError());
     ret = DeleteFileA(dest);
     ok(!ret, "DeleteFileA: error %ld\n", GetLastError());
+
+    /* test renaming file to hardlink of itself with different case */
+    lstrcpyA(dest, tempdir);
+    lstrcatA(dest, "\hardlink");
+    ret = CreateHardLinkA(dest, source, NULL);
+    ok(ret, "CreateHardLinkA: error %ld\n", GetLastError());
+    *strrchr(dest, 'l') = 'L';
+    ret = MoveFileA(source, dest);
+    ok(ret, "MoveFileA: error %ld\n", GetLastError());
+
+    hfind = FindFirstFileA(dest, &find_data);
+    ok(hfind != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
+    if (hfind != INVALID_HANDLE_VALUE)
+    {
+        todo_wine
+        ok(!lstrcmpA(strrchr(dest, '\') + 1, find_data.cFileName),
+           "MoveFile failed to change casing on hardlink of itself: got %s\n", find_data.cFileName);
+    }
+    FindClose(hfind);
+    ret = GetFileAttributesA(source);
+    todo_wine
+    ok(ret == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND, "GetFileAttributesA: error %ld\n", GetLastError());
+    if(ret != INVALID_FILE_ATTRIBUTES) DeleteFileA(source);
+    ret = DeleteFileA(dest);
+    ok(ret, "DeleteFileA: error %ld\n", GetLastError());
+
     ret = RemoveDirectoryA(tempdir);
     ok(ret, "DeleteDirectoryA: error %ld\n", GetLastError());
 }

[PATCH v13 3/4] server: Handle renames to destinations containing trailing slashes.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

They don't affect the operation even for regular files.

Signed-off-by: Gabriel Ivăncescu 
---

Also simplifies next patch.
---
 dlls/kernel32/tests/file.c | 18 ++++++++++++++++++
 server/fd.c                |  5 +++++
 2 files changed, 23 insertions(+)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index e81fa0a4e20..378c3eb12a0 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -2185,6 +2185,24 @@ static void test_MoveFileA(void)
     todo_wine
     ok(ret == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND, "GetFileAttributesA: error %ld\n", GetLastError());
     if(ret != INVALID_FILE_ATTRIBUTES) DeleteFileA(source);
+
+    /* test renaming a regular file to a name with trailing slash */
+    lstrcpyA(source, dest);
+    lstrcpyA(strrchr(dest, '\') + 1, "dir\");
+    ret = MoveFileA(source, dest);
+    ok(ret, "MoveFileA: error %ld\n", GetLastError());
+    *strrchr(dest, '\') = '\0';
+
+    hfind = FindFirstFileA(dest, &find_data);
+    ok(hfind != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %ld\n", GetLastError());
+    if (hfind != INVALID_HANDLE_VALUE)
+    {
+        ok(!lstrcmpA(find_data.cFileName, "dir"),
+           "MoveFile failed to rename regular file to name with trailing slash: got %s\n", find_data.cFileName);
+    }
+    FindClose(hfind);
+    ret = GetFileAttributesA(source);
+    ok(ret == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND, "GetFileAttributesA: error %ld\n", GetLastError());
     ret = DeleteFileA(dest);
     ok(ret, "DeleteFileA: error %ld\n", GetLastError());

diff --git a/server/fd.c b/server/fd.c
index bb0e90d99d0..4a0f7a1ad02 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -2568,6 +2568,11 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da
         set_error( STATUS_OBJECT_PATH_SYNTAX_BAD );
         return;
     }
+
+    /* strip trailing slashes */
+    while (len > 1 && nameptr[len - 1] == '/')
+        len--;
+
     if (!(name = mem_alloc( len + 1 ))) return;
     memcpy( name, nameptr, len );
     name[len] = 0;

[PATCH v13 4/4] server: Handle hardlinks and casefolding when renaming the same file.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Based on code and suggestions by Jinoh Kang.

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/kernel32/tests/file.c |   2 -
 server/fd.c                | 141 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 140 insertions(+), 3 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 378c3eb12a0..621293f8a69 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -2182,9 +2182,7 @@ static void test_MoveFileA(void)
     }
     FindClose(hfind);
     ret = GetFileAttributesA(source);
-    todo_wine
     ok(ret == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND, "GetFileAttributesA: error %ld\n", GetLastError());
-    if(ret != INVALID_FILE_ATTRIBUTES) DeleteFileA(source);

/* test renaming a regular file to a name with trailing slash */
     lstrcpyA(source, dest);
diff --git a/server/fd.c b/server/fd.c
index 4a0f7a1ad02..80517278929 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -2543,6 +2543,144 @@ static void set_fd_disposition( struct fd *fd, unsigned int flags )
         ((fd->options & FILE_DELETE_ON_CLOSE) ? FILE_DISPOSITION_DELETE : 0);
 }

+/* rename the same file, dealing with casefolding and possibly different hardlinks to it */
+static void rename_same_file( const char *src, const char *dst, int is_dir )
+{
+    static const char tmpname_fmt[] = ".wine-rename-tmp-%08x";
+    static unsigned tmp_value;
+
+    char *dirname, tmpname[sizeof(tmpname_fmt) + 4 /* remaining of %08x */];
+    int dirfd, undo_tmp_rename = 0, res = 0;
+    const char *srcname, *dstname;
+    struct stat st, st2;
+    unsigned i;
+
+    if (!strcmp( src, "/" ) || !strcmp( dst, "/" ))
+    {
+        set_error( STATUS_ACCESS_DENIED );
+        return;
+    }
+
+    /* first, check the directories they reside in */
+    dstname = strrchr( dst, '/' ) + 1;
+    if (!(dirname = memdup( dst, dstname - dst )))
+        return;
+    dirname[dstname - dst - 1] = '\0';
+    if ((res = stat( dirname, &st )))
+        file_set_error();
+    free( dirname );
+    if (res)
+        return;
+
+    srcname = strrchr( src, '/' ) + 1;
+    if (!(dirname = memdup( src, srcname - src )))
+        return;
+    dirname[srcname - src - 1] = '\0';
+    if ((dirfd = open( dirname, O_RDONLY | O_NONBLOCK )) == -1)
+        file_set_error();
+    free( dirname );
+    if (dirfd == -1)
+        return;
+
+    if (fstat( dirfd, &st2 ))
+    {
+        file_set_error();
+        goto ret;
+    }
+
+    /* if different directories, it must be a hardlink, so simply remove the source */
+    if (st.st_dev != st2.st_dev || st.st_ino != st2.st_ino)
+    {
+        if (unlinkat( dirfd, srcname, 0 ))
+            file_set_error();
+        goto ret;
+    }
+
+    /* same dentry means a no-op */
+    if (!strcmp( srcname, dstname )) goto ret;
+
+    /* This is more complicated now, because in case of a casefold (+F) directory, the destination may very well be the same dentry,
+     * even if the name doesn't match (if it differs just by case), in which case unlinking it is wrong and can be dangerous. Instead,
+     * we first rename the source to a temporary filename in the same directory. If this is a casefold dir, this will also remove the
+     * destination, otherwise the destination still exists. We then create a hardlink from the destination to our temporary name, and
+     * finally, unlink the temporary. This still works if the directory is case sensitive, so it's not a problem in either case. */
+    tmp_value += (current_time >> 16) + current_time;
+    for (i = 0; i < 0x8000; i++, tmp_value += 7777)
+    {
+        snprintf( tmpname, sizeof(tmpname), tmpname_fmt, tmp_value );
+
+        /* create an empty file or directory to avoid TOCTTOU */
+        if (!is_dir)
+        {
+            int fd = openat( dirfd, tmpname, O_CREAT | O_EXCL | O_WRONLY, 0666 );
+            if (fd == -1)
+            {
+                if (errno == EEXIST) continue;
+                file_set_error();
+                goto ret;
+            }
+            close( fd );
+        }
+        else if (mkdirat( dirfd, tmpname, 0777 ))
+        {
+            if (errno == EEXIST) continue;
+            file_set_error();
+            goto ret;
+        }
+
+        if (renameat( dirfd, srcname, dirfd, tmpname ))
+        {
+            file_set_error();
+            unlinkat( dirfd, tmpname, is_dir ? AT_REMOVEDIR : 0 );
+            goto ret;
+        }
+
+        break;
+    }
+    if (i >= 0x8000)
+    {
+        set_error( STATUS_UNSUCCESSFUL );
+        goto ret;
+    }
+
+    undo_tmp_rename = 1;
+
+    /* directories can't have hardlinks */
+    if (!is_dir)
+    {
+        /* we can't just do rename(2) here; renaming a file to its hardlink is no-op */
+        if (!linkat( dirfd, tmpname, dirfd, dstname, 0 ) || errno == EEXIST)
+        {
+            undo_tmp_rename = 0;
+
+            if (unlinkat( dirfd, tmpname, 0 ))
+                file_set_error();
+            goto ret;
+        }
+
+        if (errno != EPERM && errno != EOPNOTSUPP)
+        {
+            file_set_error();
+            goto ret;
+        }
+
+        /* filesystem doesn't support hardlinks, fall back to renaming (same as directories) */
+    }
+
+    if (renameat( dirfd, tmpname, dirfd, dstname ))
+        file_set_error();
+    else
+        undo_tmp_rename = 0;
+
+ret:
+    if (undo_tmp_rename)
+    {
+        /* revert the temporary rename */
+        renameat( dirfd, tmpname, dirfd, srcname );
+    }
+    close( dirfd );
+}
+
 /* set new name for the fd */
 static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, data_size_t len,
                          struct unicode_str nt_name, int create_link, unsigned int flags )
@@ -2600,7 +2738,8 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da
     {
         if (!fstat( fd->unix_fd, &st2 ) && st.st_ino == st2.st_ino && st.st_dev == st2.st_dev)
         {
-            if (create_link && !replace) set_error( STATUS_OBJECT_NAME_COLLISION );
+            if (!create_link) rename_same_file( fd->unix_name, name, S_ISDIR( st.st_mode ) );
+            else if (!replace) set_error( STATUS_OBJECT_NAME_COLLISION );
             free( name );
             return;
         }

[Gabriel Ivăncescu (＠insn)]

On Sun Mar 30 17:08:01 2025 +0000, Gabriel Ivăncescu wrote:
> changed this line in [version 13 of the diff](/wine/wine/-/merge_requests/6855/diffs?diff_id=167479&start_sha=dfe9be50de60d388e01ef3647f9a759b1c6f0ef5#21721cceaf2ea82f93539de2c6e01cc016bbac65_2584_2592)

Done, thanks with all the help and reviewing it.

[PATCH v13 0/4] MR6855: server: Fix renaming file to a hardlink of itself. - approved

[Jinoh Kang (＠iamahuman)]

This merge request was approved by Jinoh Kang.