[PATCH 0/2] MR10678: ntdll: Unwind the Unix path if necessary when processing relative symlinks.
From: Elizabeth Figura <zfigura@codeweavers.com> This is a case broken with the current code. Found by Esme Povirk. --- dlls/ntdll/tests/file.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index ead174f0d1f..2a32eaa3160 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -7116,6 +7116,31 @@ static void test_reparse_points(void) status = NtOpenFile( &handle2, READ_CONTROL, &attr, &io, 0, 0 ); ok( status == STATUS_IO_REPARSE_DATA_INVALID, "got %#lx\n", status ); + NtClose( handle ); + + /* Test a link that unwinds past the directory the link is in. */ + + RtlInitUnicodeString( &nameW, L"testreparse_dir\\file" ); + status = NtCreateFile( &handle, GENERIC_ALL, &attr, &io, NULL, 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + FILE_OPEN_IF, FILE_NON_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT, NULL, 0 ); + ok( !status, "got %#lx\n", status ); + + swprintf( path, ARRAY_SIZE(path), L"..\\testreparse_file", temp_path ); + data_size = init_reparse_symlink( &data, path, SYMLINK_FLAG_RELATIVE ); + status = NtFsControlFile( handle, NULL, NULL, NULL, &io, FSCTL_SET_REPARSE_POINT, data, data_size, NULL, 0 ); + ok( !status, "got %#lx\n", status ); + + status = NtOpenFile( &handle2, GENERIC_READ | SYNCHRONIZE, &attr, &io, 0, FILE_SYNCHRONOUS_IO_NONALERT ); + ok( !status, "got %#lx\n", status ); + + ret = ReadFile( handle2, buffer, sizeof(buffer), &size, NULL ); + todo_wine ok( ret == TRUE, "got error %lu\n", GetLastError() ); + todo_wine ok( size == 5, "got size %lu\n", size ); + ok( !memcmp( buffer, "file2", size ), "got data %s\n", debugstr_an( buffer, size )); + + NtClose( handle2 ); + NtClose( handle ); + /* Create an absolute symlink. */ RtlInitUnicodeString( &nameW, L"testreparse_dirlink" ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10678
From: Elizabeth Figura <zfigura@codeweavers.com> --- dlls/ntdll/tests/file.c | 2 +- dlls/ntdll/unix/file.c | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 2a32eaa3160..69aac1506e7 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -7134,7 +7134,7 @@ static void test_reparse_points(void) ok( !status, "got %#lx\n", status ); ret = ReadFile( handle2, buffer, sizeof(buffer), &size, NULL ); - todo_wine ok( ret == TRUE, "got error %lu\n", GetLastError() ); + ok( ret == TRUE, "got error %lu\n", GetLastError() ); todo_wine ok( size == 5, "got size %lu\n", size ); ok( !memcmp( buffer, "file2", size ), "got data %s\n", debugstr_an( buffer, size )); diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 3e592863f58..943af86e58f 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4014,7 +4014,8 @@ static NTSTATUS resolve_absolute_reparse_point( const WCHAR *target, unsigned in /* limited version of collapse_path() that only deals with . and .. elements * in relative symlinks */ -static NTSTATUS collapse_relative_symlink( WCHAR *path, unsigned int len, unsigned int *ret_len ) +static NTSTATUS collapse_relative_symlink( WCHAR *path, unsigned int len, unsigned int *ret_len, + unsigned int *nt_pos, const char *unix_name, int *unix_len ) { const WCHAR *end = path + len; WCHAR *p, *start, *next; @@ -4057,6 +4058,12 @@ static NTSTATUS collapse_relative_symlink( WCHAR *path, unsigned int len, unsign while (p > start && p[-1] != '\\') p--; if (p > start) p--; end = p; + if (p - path < *nt_pos) + { + while (*unix_len && unix_name[*unix_len - 1] != '/') --*unix_len; + if (*unix_len) --*unix_len; + *nt_pos = p - path; + } continue; } else if (p[2] == '\\') /* ..\ component */ @@ -4067,6 +4074,12 @@ static NTSTATUS collapse_relative_symlink( WCHAR *path, unsigned int len, unsign while (p > start && p[-1] != '\\') p--; memmove( p, next, (end - next) * sizeof(WCHAR) ); end -= (next - p); + if (p - path < *nt_pos) + { + while (*unix_len && unix_name[*unix_len - 1] != '/') --*unix_len; + if (*unix_len) --*unix_len; + *nt_pos = p - path; + } continue; } } @@ -4147,7 +4160,8 @@ static NTSTATUS resolve_reparse_point( int fd, int root_fd, OBJECT_ATTRIBUTES *a memcpy( new_nt_name, name, nt_pos * sizeof(WCHAR) ); memcpy( new_nt_name + nt_pos, target, target_len * sizeof(WCHAR) ); - if ((status = collapse_relative_symlink( new_nt_name, nt_pos + target_len, &collapsed_len ))) + if ((status = collapse_relative_symlink( new_nt_name, nt_pos + target_len, + &collapsed_len, &nt_pos, *unix_name, &pos ))) { if (attr->RootDirectory) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10678
In Unix, directory symlinks interact weirdly with `..`. If `a/b` is a symlink to `x/y`, then `a/b/..` actually points to `x`: `b` and `..` do not cancel each other. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10678#note_136434
In Unix, directory symlinks interact weirdly with `..`. If `a/b` is a symlink to `x/y`, then `a/b/..` actually points to `x`: `b` and `..` do not cancel each other.
I think you're misunderstanding what this is solving, or how it's solving it, which is my fault for not providing an explanation. Previously, when resolving a relative symlink, we do segment-by-segment resolution, as usual. Suppose we are opening a path like \??\C:\one\two\three\four where 'three' symlinks to '..\targetdir'. At the point we encounter the symlink, our half-constructed Unix path is '$WINEPREFIX/drive_c/one/two'. (We don't add 'three' because it's a reparse point.) We rewrite the NT path to \??\C:\one\targetdir\four. However, we (a) don't update the 'nt_pos' variable which points towards which segment in the NT path we're processing, and (b) don't get rid of the path segments we stripped from the Unix path. The end result is that we look for the file 'etdir' in the Unix directory '$WINEPREFIX/drive_c/one/two'. This commit fixes that by rewinding the lengths; it does not append '..' to the Unix path. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10678#note_136554
participants (3)
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Jinoh Kang (＠iamahuman)