[PATCH 0/3] MR7583: ntdll: Stop using chdir() in nt_to_unix_file_name().
From: Brendan Shanks <bshanks(a)codeweavers.com> Multiple threads could be modifying it simultaneously through nt_to_unix_file_name_no_root(). --- dlls/ntdll/unix/file.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 8af82a993e4..07b65dc23e8 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -1096,6 +1096,8 @@ static char *get_device_mount_point( dev_t dev ) #if defined(HAVE_GETATTRLIST) && defined(ATTR_VOL_CAPABILITIES) && \ defined(VOL_CAPABILITIES_FORMAT) && defined(VOL_CAP_FMT_CASE_SENSITIVE) +static pthread_mutex_t fs_cache_mutex = PTHREAD_MUTEX_INITIALIZER; + struct get_fsid { ULONG size; @@ -1172,6 +1174,7 @@ static void add_fs_cache( dev_t dev, fsid_t fsid, BOOLEAN case_sensitive ) */ static int get_dir_case_sensitivity_attr( const char *dir ) { + BOOLEAN ret = FALSE; char *mntpoint; struct attrlist attr; struct vol_caps caps; @@ -1187,13 +1190,18 @@ static int get_dir_case_sensitivity_attr( const char *dir ) if (getattrlist( dir, &attr, &get_fsid, sizeof(get_fsid), 0 ) != 0 || get_fsid.size != sizeof(get_fsid)) return -1; + /* Try to look it up in the cache */ + mutex_lock( &fs_cache_mutex ); entry = look_up_fs_cache( get_fsid.dev ); if (entry && !memcmp( &entry->fsid, &get_fsid.fsid, sizeof(fsid_t) )) + { /* Cache lookup succeeded */ - return entry->case_sensitive; - /* Cache is stale at this point, we have to update it */ + ret = entry->case_sensitive; + goto done; + } + /* Cache is stale at this point, we have to update it */ mntpoint = get_device_mount_point( get_fsid.dev ); /* Now look up the case-sensitivity */ attr.commonattr = 0; @@ -1202,7 +1210,8 @@ static int get_dir_case_sensitivity_attr( const char *dir ) { free( mntpoint ); add_fs_cache( get_fsid.dev, get_fsid.fsid, TRUE ); - return TRUE; + ret = TRUE; + goto done; } free( mntpoint ); if (caps.size == sizeof(caps) && @@ -1210,8 +1219,6 @@ static int get_dir_case_sensitivity_attr( const char *dir ) (VOL_CAP_FMT_CASE_SENSITIVE | VOL_CAP_FMT_CASE_PRESERVING)) == (VOL_CAP_FMT_CASE_SENSITIVE | VOL_CAP_FMT_CASE_PRESERVING)) { - BOOLEAN ret; - if ((caps.caps.capabilities[VOL_CAPABILITIES_FORMAT] & VOL_CAP_FMT_CASE_SENSITIVE) != VOL_CAP_FMT_CASE_SENSITIVE) ret = FALSE; @@ -1219,9 +1226,12 @@ static int get_dir_case_sensitivity_attr( const char *dir ) ret = TRUE; /* Update the cache */ add_fs_cache( get_fsid.dev, get_fsid.fsid, ret ); - return ret; + goto done; } - return FALSE; + +done: + mutex_unlock( &fs_cache_mutex ); + return ret; } #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7583
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 07b65dc23e8..649434f0a56 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -1172,7 +1172,7 @@ static void add_fs_cache( dev_t dev, fsid_t fsid, BOOLEAN case_sensitive ) * Checks if the volume containing the specified directory is case * sensitive or not. Uses getattrlist(2). */ -static int get_dir_case_sensitivity_attr( const char *dir ) +static int get_dir_case_sensitivity_attr( int root_fd, const char *dir ) { BOOLEAN ret = FALSE; char *mntpoint; @@ -1187,7 +1187,7 @@ static int get_dir_case_sensitivity_attr( const char *dir ) attr.commonattr = ATTR_CMN_DEVID|ATTR_CMN_FSID; attr.volattr = attr.dirattr = attr.fileattr = attr.forkattr = 0; get_fsid.size = 0; - if (getattrlist( dir, &attr, &get_fsid, sizeof(get_fsid), 0 ) != 0 || + if (getattrlistat( root_fd, dir, &attr, &get_fsid, sizeof(get_fsid), 0 ) != 0 || get_fsid.size != sizeof(get_fsid)) return -1; @@ -1241,12 +1241,19 @@ done: * Checks if the volume containing the specified directory is case * sensitive or not. Uses (f)statfs(2), statvfs(2), fstatat(2), or ioctl(2). */ -static BOOLEAN get_dir_case_sensitivity_stat( const char *dir ) +static BOOLEAN get_dir_case_sensitivity_stat( int root_fd, const char *dir ) { #if defined(__APPLE__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__) struct statfs stfs; + int fd; - if (statfs( dir, &stfs ) == -1) return TRUE; + if ((fd = openat( root_fd, dir, O_RDONLY )) == -1) return TRUE; + if (fstatfs( fd, &stfs ) == -1) + { + close( fd ); + return TRUE; + } + close( fd ); /* Assume these file systems are always case insensitive.*/ if (!strcmp( stfs.f_fstypename, "fusefs" ) && !strncmp( stfs.f_mntfromname, "ciopfs", 5 )) @@ -1287,8 +1294,15 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir ) #elif defined(__NetBSD__) struct statvfs stfs; + int fd; - if (statvfs( dir, &stfs ) == -1) return TRUE; + if ((fd = openat( root_fd, dir, O_RDONLY )) == -1) return TRUE; + if (fstatvfs( fd, &stfs ) == -1) + { + close( fd ); + return TRUE; + } + close( fd ); /* Only assume CIOPFS is case insensitive. */ if (strcmp( stfs.f_fstypename, "fusefs" ) || strncmp( stfs.f_mntfromname, "ciopfs", 5 )) @@ -1301,7 +1315,7 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir ) struct stat st; int fd, flags; - if ((fd = open( dir, O_RDONLY | O_NONBLOCK )) == -1) + if ((fd = openat( root_fd, dir, O_RDONLY | O_NONBLOCK )) == -1) return TRUE; if (ioctl( fd, EXT2_IOC_GETFLAGS, &flags ) != -1 && (flags & EXT4_CASEFOLD_FL)) @@ -1329,14 +1343,14 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir ) * Checks if the volume containing the specified directory is case * sensitive or not. Uses multiple methods, depending on platform. */ -static BOOLEAN get_dir_case_sensitivity( const char *dir ) +static BOOLEAN get_dir_case_sensitivity( int root_fd, const char *dir ) { #if defined(HAVE_GETATTRLIST) && defined(ATTR_VOL_CAPABILITIES) && \ defined(VOL_CAPABILITIES_FORMAT) && defined(VOL_CAP_FMT_CASE_SENSITIVE) - int case_sensitive = get_dir_case_sensitivity_attr( dir ); + int case_sensitive = get_dir_case_sensitivity_attr( root_fd, dir ); if (case_sensitive != -1) return case_sensitive; #endif - return get_dir_case_sensitivity_stat( dir ); + return get_dir_case_sensitivity_stat( root_fd, dir ); } @@ -2539,7 +2553,7 @@ static NTSTATUS read_directory_data_stat( struct dir_data *data, const char *uni struct stat st; /* if the file system is not case sensitive we can't find the actual name through stat() */ - if (!get_dir_case_sensitivity(".")) return STATUS_NO_SUCH_FILE; + if (!get_dir_case_sensitivity( AT_FDCWD, "." )) return STATUS_NO_SUCH_FILE; if (stat( unix_name, &st ) == -1) return STATUS_NO_SUCH_FILE; TRACE( "found %s\n", debugstr_a(unix_name) ); @@ -2894,7 +2908,7 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i is_name_8_dot_3 = is_name_8_dot_3 && length >= 8 && name[4] == '~'; #endif - if (!is_name_8_dot_3 && !get_dir_case_sensitivity( unix_name )) goto not_found; + if (!is_name_8_dot_3 && !get_dir_case_sensitivity( AT_FDCWD, unix_name )) goto not_found; /* now look for it through the directory */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7583
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 649434f0a56..1f676c511db 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -2877,7 +2877,7 @@ NTSTATUS WINAPI NtQueryDirectoryFile( HANDLE handle, HANDLE event, PIO_APC_ROUTI * The file found is appended to unix_name at pos. * There must be at least MAX_DIR_ENTRY_LEN+2 chars available at pos. */ -static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, int length, +static NTSTATUS find_file_in_dir( int root_fd, char *unix_name, int pos, const WCHAR *name, int length, BOOLEAN check_case ) { WCHAR buffer[MAX_DIR_ENTRY_LEN]; @@ -2885,7 +2885,7 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i DIR *dir; struct dirent *de; struct stat st; - int ret; + int fd, ret; /* try a shortcut for this directory */ @@ -2894,7 +2894,7 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i if (ret >= 0 && ret <= MAX_DIR_ENTRY_LEN) { unix_name[pos + ret] = 0; - if (!stat( unix_name, &st )) return STATUS_SUCCESS; + if (!fstatat( root_fd, unix_name, &st, 0 )) return STATUS_SUCCESS; } if (check_case) goto not_found; /* we want an exact match */ @@ -2908,14 +2908,14 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i is_name_8_dot_3 = is_name_8_dot_3 && length >= 8 && name[4] == '~'; #endif - if (!is_name_8_dot_3 && !get_dir_case_sensitivity( AT_FDCWD, unix_name )) goto not_found; + if (!is_name_8_dot_3 && !get_dir_case_sensitivity( root_fd, unix_name )) goto not_found; /* now look for it through the directory */ #ifdef VFAT_IOCTL_READDIR_BOTH if (is_name_8_dot_3) { - int fd = open( unix_name, O_RDONLY | O_DIRECTORY ); + int fd = openat( root_fd, unix_name, O_RDONLY | O_DIRECTORY ); if (fd != -1) { KERNEL_DIRENT kde[2]; @@ -2960,7 +2960,12 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i } #endif /* VFAT_IOCTL_READDIR_BOTH */ - if (!(dir = opendir( unix_name ))) return errno_to_status( errno ); + if ((fd = openat( root_fd, unix_name, O_RDONLY )) == -1) return errno_to_status( errno ); + if (!(dir = fdopendir( fd ))) + { + close( fd ); + return errno_to_status( errno ); + } unix_name[pos - 1] = '/'; while ((de = readdir( dir ))) @@ -3512,8 +3517,8 @@ done: * * Helper for nt_to_unix_file_name */ -static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer, int unix_len, int pos, - UINT disposition, BOOL is_unix ) +static NTSTATUS lookup_unix_name( int root_fd, const WCHAR *name, int name_len, char **buffer, int unix_len, + int pos, UINT disposition, BOOL is_unix ) { static const WCHAR invalid_charsW[] = { INVALID_NT_CHARS, '/', 0 }; NTSTATUS status; @@ -3555,7 +3560,7 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer char *p; unix_name[pos + 1 + ret] = 0; for (p = unix_name + pos ; *p; p++) if (*p == '\\') *p = '/'; - if (!stat( unix_name, &st )) + if (!fstatat( root_fd, unix_name, &st, 0 )) { if (disposition == FILE_CREATE) return STATUS_OBJECT_NAME_COLLISION; return STATUS_SUCCESS; @@ -3589,7 +3594,7 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer unix_name = *buffer = new_name; } - status = find_file_in_dir( unix_name, pos, name, end - name, is_unix ); + status = find_file_in_dir( root_fd, unix_name, pos, name, end - name, is_unix ); /* if this is the last element, not finding it is not necessarily fatal */ if (!name_len) @@ -3726,7 +3731,7 @@ static NTSTATUS nt_to_unix_file_name_no_root( const UNICODE_STRING *nameW, char name += prefix_len; name_len -= prefix_len; - status = lookup_unix_name( name, name_len, &unix_name, unix_len, pos, disposition, is_unix ); + status = lookup_unix_name( AT_FDCWD, name, name_len, &unix_name, unix_len, pos, disposition, is_unix ); if (status == STATUS_SUCCESS || status == STATUS_NO_SUCH_FILE) { TRACE( "%s -> %s\n", debugstr_us(nameW), debugstr_a(unix_name) ); @@ -3753,7 +3758,7 @@ static NTSTATUS nt_to_unix_file_name_no_root( const UNICODE_STRING *nameW, char NTSTATUS nt_to_unix_file_name( const OBJECT_ATTRIBUTES *attr, char **name_ret, UINT disposition ) { enum server_fd_type type; - int old_cwd, root_fd, needs_close; + int root_fd, needs_close; const WCHAR *name; char *unix_name; int name_len, unix_len; @@ -3780,15 +3785,7 @@ NTSTATUS nt_to_unix_file_name( const OBJECT_ATTRIBUTES *attr, char **name_ret, U } else { - mutex_lock( &dir_mutex ); - if ((old_cwd = open( ".", O_RDONLY )) != -1 && fchdir( root_fd ) != -1) - { - status = lookup_unix_name( name, name_len, &unix_name, unix_len, 1, disposition, FALSE ); - if (fchdir( old_cwd ) == -1) chdir( "/" ); - } - else status = errno_to_status( errno ); - mutex_unlock( &dir_mutex ); - if (old_cwd != -1) close( old_cwd ); + status = lookup_unix_name( root_fd, name, name_len, &unix_name, unix_len, 1, disposition, FALSE ); if (needs_close) close( root_fd ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7583
Jinoh Kang (@iamahuman) commented about dlls/ntdll/unix/file.c:
* Checks if the volume containing the specified directory is case * sensitive or not. Uses getattrlist(2).
* sensitive or not. Uses getattrlistat(2).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7583#note_97894
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
Jinoh Kang (＠iamahuman)