[PATCH v2 0/4] MR8688: server: Start to expose Unix symlinks as NT reparse points.
A Wine user has requested the ability to interact with Unix symlinks from Win32 code. While the generic functionality of NT reparse points is still not implemented upstream, it will probably involve a completely different code path than this, so there does not seem to me to be any point in waiting for that to be done before submitting these patches. This does not implement all the relevant functionality. FILE_OPEN_REPARSE_POINT is now respected, but returns an fd with no fd->unix_fd. Thus far only file disposition (i.e. deletion), renaming and hardlinking are implemented, and all other operations return the no_fd_status, for which STATUS_REPARSE_POINT_NOT_RESOLVED is arbitrarily chosen. This is probably going to be controversial. As controversial patches tend to get deferred forever, I'm going to at least try to pose some questions related to concept and implementation which may be easier to answer. * Do we want to expose Unix symlinks at all? This was requested by a Wine user, and it does seem a strong argument that Wine, in general, supports some measure of integration with the host, including exposing custom interfaces for programs written to take advantage of it. We also, currently, already expose Unix *directory* symlinks via NtQueryAttributesFile(). We don't currently expose such file symlinks (and this series does not change that particular point, but it would be done by follow-up patches). In a sense we are expanding existing partial support. On the other hand, Unix symlinks which are completely opaque can be useful, and some downstream distributions (CrossOver and Proton) currently rely on them to reduce disk space usage while keeping the application unaware of a symlink (even if it were to use FILE_OPEN_REPARSE_POINT. Not that any applications are currently known that would break if this were to change, and in any case they could simply revert these patches.) * Implementation-wise, is having unix_fd == -1 a non-starter? This can be true of other real fds in some specific cases... * It's also true that this current implementation suffers from some degree of TOCTOU problems: because we only store the name and not the FD anymore, the actual underlying object at that path can change. This would not be true on Windows. Is this a problem? * One alternate approach would be to use O_PATH (Linux, FreeBSD) and O_SYMLINK (Mac). I don't see any equivalent flag for NetBSD or Solaris, so we would probably just not be able to expose this functionality there. Note that while the documentation seems to imply that O_PATH only stores the path, manual testing shows that (at least on Linux) a fd opened with O_PATH will remain pointing at the same *inode* even if it is moved or deleted. So this would avoid the aforementioned TOCTOU problem. -- v2: server: Allow renaming and hardlinking Unix symlinks. server: Allow setting a disposition on Unix symlinks. server: Allow opening Unix symlinks with FILE_OPEN_REPARSE_POINT. server: Separate a open_unix_fd() helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/8688
From: Elizabeth Figura <zfigura@codeweavers.com> --- server/fd.c | 59 +++++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/server/fd.c b/server/fd.c index 4c0aa36d359..0ce28578e1e 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1919,6 +1919,36 @@ void get_nt_name( struct fd *fd, struct unicode_str *name ) name->len = fd->nt_namelen; } +static int open_unix_fd( struct fd *fd, const char *name, int flags, const mode_t *mode, + unsigned int access, unsigned int options ) +{ + int rw_mode; + + if ((access & FILE_UNIX_WRITE_ACCESS) && !(options & FILE_DIRECTORY_FILE)) + { + if (access & FILE_UNIX_READ_ACCESS) rw_mode = O_RDWR; + else rw_mode = O_WRONLY; + } + else rw_mode = O_RDONLY; + + if ((fd->unix_fd = open( name, rw_mode | (flags & ~O_TRUNC), *mode )) != -1) + return 1; + + /* if we tried to open a directory for write access, retry read-only */ + if (errno == EISDIR && ((access & FILE_UNIX_WRITE_ACCESS) || (flags & O_CREAT))) + { + if ((fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ))) + return 1; + } + + /* check for trailing slash on file path */ + if ((errno == ENOENT || (errno == ENOTDIR && !(options & FILE_DIRECTORY_FILE))) && name[strlen(name) - 1] == '/') + set_error( STATUS_OBJECT_NAME_INVALID ); + else + file_set_error(); + return 0; +} + /* open() wrapper that returns a struct fd with no fd user set */ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_name, int flags, mode_t *mode, unsigned int access, @@ -1928,7 +1958,6 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam struct closed_fd *closed_fd; struct fd *fd; int root_fd = -1; - int rw_mode; char *path; if (((options & FILE_DELETE_ON_CLOSE) && !(access & DELETE)) || @@ -1972,32 +2001,8 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam flags &= ~(O_CREAT | O_EXCL | O_TRUNC); } - if ((access & FILE_UNIX_WRITE_ACCESS) && !(options & FILE_DIRECTORY_FILE)) - { - if (access & FILE_UNIX_READ_ACCESS) rw_mode = O_RDWR; - else rw_mode = O_WRONLY; - } - else rw_mode = O_RDONLY; - - if ((fd->unix_fd = open( name, rw_mode | (flags & ~O_TRUNC), *mode )) == -1) - { - /* if we tried to open a directory for write access, retry read-only */ - if (errno == EISDIR) - { - if ((access & FILE_UNIX_WRITE_ACCESS) || (flags & O_CREAT)) - fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); - } - - if (fd->unix_fd == -1) - { - /* check for trailing slash on file path */ - if ((errno == ENOENT || (errno == ENOTDIR && !(options & FILE_DIRECTORY_FILE))) && name[strlen(name) - 1] == '/') - set_error( STATUS_OBJECT_NAME_INVALID ); - else - file_set_error(); - goto error; - } - } + if (!open_unix_fd( fd, name, flags, mode, access, options )) + goto error; fd->nt_name = dup_nt_name( root, nt_name, &fd->nt_namelen ); fd->unix_name = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8688
From: Elizabeth Figura <zfigura@codeweavers.com> --- server/fd.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/server/fd.c b/server/fd.c index 0ce28578e1e..322256cf510 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1941,6 +1941,12 @@ static int open_unix_fd( struct fd *fd, const char *name, int flags, const mode_ return 1; } + if (errno == ELOOP && (options & FILE_OPEN_REPARSE_POINT)) + { + fd->no_fd_status = STATUS_REPARSE_POINT_NOT_RESOLVED; + return 1; + } + /* check for trailing slash on file path */ if ((errno == ENOENT || (errno == ENOTDIR && !(options & FILE_DIRECTORY_FILE))) && name[strlen(name) - 1] == '/') set_error( STATUS_OBJECT_NAME_INVALID ); @@ -1949,6 +1955,31 @@ static int open_unix_fd( struct fd *fd, const char *name, int flags, const mode_ return 0; } +/* resolve the path except for its last element */ +static char *symlink_realpath( const char *path ) +{ + size_t len = strlen( path ); + char *dir, *ret; + + /* remove trailing slashes if any */ + while (len > 0 && path[len - 1] == '/') + --len; + + while (len > 0 && path[len - 1] != '/') + --len; + + dir = malloc( len + 1 ); + memcpy( dir, path, len ); + dir[len] = 0; + ret = realpath( dir, NULL ); + free( dir ); + + ret = realloc( ret, strlen( ret ) + 1 + strlen( path + len ) + 1 ); + strcat( ret, "/" ); + strcat( ret, path + len ); + return ret; +} + /* open() wrapper that returns a struct fd with no fd user set */ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_name, int flags, mode_t *mode, unsigned int access, @@ -2001,16 +2032,28 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam flags &= ~(O_CREAT | O_EXCL | O_TRUNC); } + if (options & FILE_OPEN_REPARSE_POINT) + flags |= O_NOFOLLOW; + if (!open_unix_fd( fd, name, flags, mode, access, options )) goto error; fd->nt_name = dup_nt_name( root, nt_name, &fd->nt_namelen ); fd->unix_name = NULL; - fstat( fd->unix_fd, &st ); + if (fd->unix_fd != -1) + fstat( fd->unix_fd, &st ); + else + { + if (lstat( name, &st ) == -1) + { + file_set_error(); + goto error; + } + } *mode = st.st_mode; /* only bother with an inode for normal files and directories */ - if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode)) + if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)) { unsigned int err; struct inode *inode = get_inode( st.st_dev, st.st_ino, fd->unix_fd ); @@ -2025,7 +2068,10 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam if ((path = dup_fd_name( root, name ))) { - fd->unix_name = realpath( path, NULL ); + if (S_ISLNK(st.st_mode)) + fd->unix_name = symlink_realpath( path ); + else + fd->unix_name = realpath( path, NULL ); free( path ); } @@ -2039,7 +2085,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam closed_fd = NULL; /* check directory options */ - if ((options & FILE_DIRECTORY_FILE) && !S_ISDIR(st.st_mode)) + if ((options & FILE_DIRECTORY_FILE) && S_ISREG(st.st_mode)) { set_error( STATUS_NOT_A_DIRECTORY ); goto error; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8688
From: Elizabeth Figura <zfigura@codeweavers.com> --- server/fd.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/server/fd.c b/server/fd.c index 322256cf510..bee6be524b5 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1103,7 +1103,7 @@ static void unlink_closed_fd( struct inode *inode, struct closed_fd *fd ) { /* make sure it is still the same file */ struct stat st; - if (!stat( fd->unix_name, &st ) && st.st_dev == inode->device->dev && st.st_ino == inode->ino) + if (!lstat( fd->unix_name, &st ) && st.st_dev == inode->device->dev && st.st_ino == inode->ino) { if (S_ISDIR(st.st_mode)) rmdir( fd->unix_name ); else unlink( fd->unix_name ); @@ -2830,15 +2830,10 @@ static void set_fd_disposition( struct fd *fd, unsigned int flags ) return; } - if (fd->unix_fd == -1) - { - set_error( fd->no_fd_status ); - return; - } - if (flags & FILE_DISPOSITION_DELETE) { struct fd *fd_ptr; + int ret; LIST_FOR_EACH_ENTRY( fd_ptr, &fd->inode->open, struct fd, inode_entry ) { @@ -2849,12 +2844,16 @@ static void set_fd_disposition( struct fd *fd, unsigned int flags ) } } - if (fstat( fd->unix_fd, &st ) == -1) + if (fd->unix_fd != -1) + ret = fstat( fd->unix_fd, &st ); + else + ret = lstat( fd->unix_name, &st ); + if (ret == -1) { file_set_error(); return; } - if (S_ISREG( st.st_mode )) /* can't unlink files we don't have permission to write */ + if (S_ISREG( st.st_mode ) || S_ISLNK( st.st_mode )) /* can't unlink files we don't have permission to write */ { if (!(flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) && !(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH))) @@ -2865,6 +2864,11 @@ static void set_fd_disposition( struct fd *fd, unsigned int flags ) } else if (S_ISDIR( st.st_mode )) /* can't remove non-empty directories */ { + if (fd->unix_fd == -1) + { + set_error( fd->no_fd_status ); + return; + } switch (is_dir_empty( fd->unix_fd )) { case -1: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8688
From: Elizabeth Figura <zfigura@codeweavers.com> --- server/fd.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/server/fd.c b/server/fd.c index bee6be524b5..4f2dddef335 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2902,17 +2902,13 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da struct stat st, st2; char *name; const unsigned int replace = flags & FILE_RENAME_REPLACE_IF_EXISTS; + int ret; if (!fd->inode || !fd->unix_name) { set_error( STATUS_OBJECT_TYPE_MISMATCH ); return; } - if (fd->unix_fd == -1) - { - set_error( fd->no_fd_status ); - return; - } if (!len || ((nameptr[0] == '/') ^ !root)) { @@ -2938,15 +2934,26 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da } /* when creating a hard link, source cannot be a dir */ - if (create_link && !fstat( fd->unix_fd, &st ) && S_ISDIR( st.st_mode )) + if (create_link) { - set_error( STATUS_FILE_IS_A_DIRECTORY ); - goto failed; + if (fd->unix_fd != -1) + ret = fstat( fd->unix_fd, &st ); + else + ret = lstat( fd->unix_name, &st ); + if (!ret && S_ISDIR( st.st_mode )) + { + set_error( STATUS_FILE_IS_A_DIRECTORY ); + goto failed; + } } if (!stat( name, &st )) { - if (!fstat( fd->unix_fd, &st2 ) && st.st_ino == st2.st_ino && st.st_dev == st2.st_dev) + if (fd->unix_fd != -1) + ret = fstat( fd->unix_fd, &st2 ); + else + ret = lstat( fd->unix_name, &st2 ); + if (!ret && st.st_ino == st2.st_ino && st.st_dev == st2.st_dev) { if (create_link && !replace) set_error( STATUS_OBJECT_NAME_COLLISION ); free( name ); @@ -3012,7 +3019,8 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da goto failed; } - if (is_file_executable( fd->unix_name ) != is_file_executable( name ) && !fstat( fd->unix_fd, &st )) + if (is_file_executable( fd->unix_name ) != is_file_executable( name ) && + fd->unix_fd != -1 && !fstat( fd->unix_fd, &st )) { if (is_file_executable( name )) /* set executable bit where read bit is set */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8688
On Mon Mar 16 19:02:40 2026 +0000, Elizabeth Figura wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/8688/diffs?diff_id=251302&start_sha=cad185ea8369c305c624d622711ad732af1885e4#21721cceaf2ea82f93539de2c6e01cc016bbac65_1971_1977) Fixed, sorry this fell by the wayside. Also, rebased.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8688#note_132399
I'm still not convinced that this is a good idea. Particularly now that we have actual reparse points, it seems to me that if there's a need for a Windows app to be able to interact with symlinks they could be created as Windows symlinks instead of Unix ones. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8688#note_132873
I'm still not convinced that this is a good idea. Particularly now that we have actual reparse points, it seems to me that if there's a need for a Windows app to be able to interact with symlinks they could be created as Windows symlinks instead of Unix ones.
Thanks, closing then. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8688#note_133597
Thanks, closing then.
@zfigura, did you mean to close this merge request per your note above? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8688#note_139370
participants (4)
-
Alexandre Julliard (＠julliard) -
Elizabeth Figura
-
Elizabeth Figura (＠zfigura)
-
Gerald Pfeifer (＠gerald)