* vboxsharedfs - Too many levels of symbolic links @ 2021-11-30 17:04 Oskar Skog 2021-11-30 21:40 ` Gackle, Philip P ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Oskar Skog @ 2021-11-30 17:04 UTC (permalink / raw) To: cygwin vboxsharedfs file systems no longer work. Any attempt to access will result in "too many levels of symbolic links". This only affects the VirtualBox shared folder, the Samba share works just fine. Last time I updated (before today) was sometime before the 10th of September. MSYS2 has the same problem, but no one seems to have reported it to Cygwin: https://github.com/msys2/msys2-runtime/issues/58 user@DESKTOP-******* ~$ ls /cygdrive/z ls: cannot access '/cygdrive/z': Too many levels of symbolic links user@DESKTOP-******* ~$ mount C:/Cygwin/bin on /usr/bin type ntfs (binary,auto) C:/Cygwin/lib on /usr/lib type ntfs (binary,auto) C:/Cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) Y: on /cygdrive/y type smbfs (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) user@DESKTOP-******* ~$ uname -a CYGWIN_NT-10.0 DESKTOP-* 3.3.2(0.341/5/3) 2021-11-08 16:55 x86_64 Cygwin ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: vboxsharedfs - Too many levels of symbolic links 2021-11-30 17:04 vboxsharedfs - Too many levels of symbolic links Oskar Skog @ 2021-11-30 21:40 ` Gackle, Philip P 2021-12-02 10:43 ` Corinna Vinschen 2021-12-05 2:54 ` Takashi Yano 2 siblings, 0 replies; 19+ messages in thread From: Gackle, Philip P @ 2021-11-30 21:40 UTC (permalink / raw) To: cygwin Same problem for me against AFS file system beyond 5 levels in the directory tree. Fell back to 3.2.0-1 (Cygwin) to resolve. None of the 3.3 versions worked (up to 3.3.2-1) -----Original Message----- From: Cygwin <cygwin-bounces+philip.gackle=pnl.gov@cygwin.com> On Behalf Of Oskar Skog Sent: Tuesday, November 30, 2021 9:05 AM To: cygwin@cygwin.com Subject: vboxsharedfs - Too many levels of symbolic links Check twice before you click! This email originated from outside PNNL. vboxsharedfs file systems no longer work. Any attempt to access will result in "too many levels of symbolic links". This only affects the VirtualBox shared folder, the Samba share works just fine. Last time I updated (before today) was sometime before the 10th of September. MSYS2 has the same problem, but no one seems to have reported it to Cygwin: https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmsys2%2Fmsys2-runtime%2Fissues%2F58&data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765066642%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=FQ%2FLAAfIRvLXr7gL1iqVQkspgWXL2WY59FB8fvx8DqI%3D&reserved=0 user@DESKTOP-******* ~$ ls /cygdrive/z ls: cannot access '/cygdrive/z': Too many levels of symbolic links user@DESKTOP-******* ~$ mount C:/Cygwin/bin on /usr/bin type ntfs (binary,auto) C:/Cygwin/lib on /usr/lib type ntfs (binary,auto) C:/Cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) Y: on /cygdrive/y type smbfs (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) user@DESKTOP-******* ~$ uname -a CYGWIN_NT-10.0 DESKTOP-* 3.3.2(0.341/5/3) 2021-11-08 16:55 x86_64 Cygwin -- Problem reports: https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Fproblems.html&data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=OUOovyeBCPL1b6mSQOHHI3k0GBQEETJV71wPfhYUltU%3D&reserved=0 FAQ: https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Ffaq%2F&data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8mQ%2FNyYJGT5J5P3pLOtu1FWQaoPWQIHZgXmfaeeI4WM%3D&reserved=0 Documentation: https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Fdocs.html&data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=l8lbrBZTWc7wJIsPLKI%2Bvsj4zzW%2Fi9RNFNvXQge%2B%2FCI%3D&reserved=0 Unsubscribe info: https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Fml%2F%23unsubscribe-simple&data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=wtlkvSXvzCzYKQlO2voUxYgq7DoiQ5Lte08rLrAw9m4%3D&reserved=0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-11-30 17:04 vboxsharedfs - Too many levels of symbolic links Oskar Skog 2021-11-30 21:40 ` Gackle, Philip P @ 2021-12-02 10:43 ` Corinna Vinschen 2021-12-05 2:54 ` Takashi Yano 2 siblings, 0 replies; 19+ messages in thread From: Corinna Vinschen @ 2021-12-02 10:43 UTC (permalink / raw) To: cygwin On Nov 30 19:04, Oskar Skog wrote: > vboxsharedfs file systems no longer work. Any attempt to access will > result in "too many levels of symbolic links". > > This only affects the VirtualBox shared folder, the Samba share works > just fine. > > Last time I updated (before today) was sometime before the 10th of > September. > > MSYS2 has the same problem, but no one seems to have reported it to > Cygwin: > https://github.com/msys2/msys2-runtime/issues/58 > > > user@DESKTOP-******* ~$ ls /cygdrive/z > ls: cannot access '/cygdrive/z': Too many levels of symbolic links > user@DESKTOP-******* ~$ mount > C:/Cygwin/bin on /usr/bin type ntfs (binary,auto) > C:/Cygwin/lib on /usr/lib type ntfs (binary,auto) > C:/Cygwin on / type ntfs (binary,auto) > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > Y: on /cygdrive/y type smbfs (binary,posix=0,user,noumount,auto) > Z: on /cygdrive/z type vboxsharedfolderfs > (binary,posix=0,user,noumount,auto) > user@DESKTOP-******* ~$ uname -a > CYGWIN_NT-10.0 DESKTOP-* 3.3.2(0.341/5/3) 2021-11-08 16:55 x86_64 Cygwin Somebody will have to debug this. From the MSYS issue it looks like the GetFinalPathNameByHandleW call results in some trouble, but the MSYS guys rather just disable code instead of trying to debug it and fixing the cause. "Somebody" would have to be somebody who can reproduce the issue. I'm neither using VirtualBox, nor AFS. I probably could test this if somebody gives me temporary access to an existing AFS FS somewhere, plus a short description how to connect. I guess it starts with "Install latest OpenAFS version x.y for Windows"... Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-11-30 17:04 vboxsharedfs - Too many levels of symbolic links Oskar Skog 2021-11-30 21:40 ` Gackle, Philip P 2021-12-02 10:43 ` Corinna Vinschen @ 2021-12-05 2:54 ` Takashi Yano 2021-12-05 14:49 ` Oskar Skog 2021-12-06 10:16 ` Corinna Vinschen 2 siblings, 2 replies; 19+ messages in thread From: Takashi Yano @ 2021-12-05 2:54 UTC (permalink / raw) To: cygwin On Tue, 30 Nov 2021 19:04:57 +0200 Oskar Skog wrote: > vboxsharedfs file systems no longer work. Any attempt to access will > result in "too many levels of symbolic links". > > This only affects the VirtualBox shared folder, the Samba share works > just fine. > > Last time I updated (before today) was sometime before the 10th of > September. > > MSYS2 has the same problem, but no one seems to have reported it to > Cygwin: > https://github.com/msys2/msys2-runtime/issues/58 > > > user@DESKTOP-******* ~$ ls /cygdrive/z > ls: cannot access '/cygdrive/z': Too many levels of symbolic links > user@DESKTOP-******* ~$ mount > C:/Cygwin/bin on /usr/bin type ntfs (binary,auto) > C:/Cygwin/lib on /usr/lib type ntfs (binary,auto) > C:/Cygwin on / type ntfs (binary,auto) > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > Y: on /cygdrive/y type smbfs (binary,posix=0,user,noumount,auto) > Z: on /cygdrive/z type vboxsharedfolderfs > (binary,posix=0,user,noumount,auto) > user@DESKTOP-******* ~$ uname -a > CYGWIN_NT-10.0 DESKTOP-* 3.3.2(0.341/5/3) 2021-11-08 16:55 x86_64 Cygwin Thanks for the report. It seems that this happens only in 64bit Windoes10/11. In 32bit Windows10: $ ls -l /cygdrive/z total 0 -rw-r--r-- 1 yano yano 0 Dec 3 22:16 testfile.txt $ mount C:/cygwin/bin on /usr/bin type ntfs (binary,auto) C:/cygwin/lib on /usr/lib type ntfs (binary,auto) C:/cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) $ uname -a CYGWIN_NT-10.0 DESKTOP-HGUT5FP 3.3.2(0.341/5/3) 2021-11-08 16:51 i686 Cygwin In 64bit Windows10: $ ls -l /cygdrive/z ls: /cygdrive/z: Too many levels of symbolic links ls: cannot open directory '/cygdrive/z': Too many levels of symbolic links $ mount C:/cygwin/bin on /usr/bin type ntfs (binary,auto) C:/cygwin/lib on /usr/lib type ntfs (binary,auto) C:/cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) $ uname -a CYGWIN_NT-10.0-WOW DESKTOP-CUHC1NV 3.3.2(0.341/5/3) 2021-11-08 16:51 i686 Cygwin I tested with VirtualBox version 6.1.30. In 64bit Windows10, for vbox shared path, GetFinalPathNameByHandleW() returns path with trailing '\'. As a result, RtlEqualUnicodeString() fails and tries to resolve symlink repeatedly. For example, RtlEqualUnicodeString() compares \??\UNC\VBoxSrv\tmp and \??\UNC\VBoxSrv\tmp\, then it fails. This does not happen in 32bit Windows10. It seems that UNC path is not treated as a symlink in 32bit Windows10. I am not sure why. Therefore, I have applied a patch which stops to treat UNC path as a symlink for testing as follows. diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index baf04ce89..31a96ca58 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -3490,6 +3490,9 @@ restart: ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0); if (ret) { + if (wcsstr (fpbuf, L"\\\\?\\UNC\\") == fpbuf) + goto file_not_symlink; + UNICODE_STRING fpath; RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); I have confirmed this patch fixes the issue. In addition, this patch also resolves the issue: https://cygwin.com/pipermail/cygwin/2021-December/250103.html Is this the right thing? -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-05 2:54 ` Takashi Yano @ 2021-12-05 14:49 ` Oskar Skog 2021-12-06 3:31 ` Takashi Yano 2021-12-06 10:16 ` Corinna Vinschen 1 sibling, 1 reply; 19+ messages in thread From: Oskar Skog @ 2021-12-05 14:49 UTC (permalink / raw) To: cygwin On 2021-12-05 04:54, Takashi Yano wrote: > On Tue, 30 Nov 2021 19:04:57 +0200 > Oskar Skog wrote: >> vboxsharedfs file systems no longer work. Any attempt to access will >> result in "too many levels of symbolic links". >> >> This only affects the VirtualBox shared folder, the Samba share works >> just fine. .... > > I tested with VirtualBox version 6.1.30. > > In 64bit Windows10, for vbox shared path, GetFinalPathNameByHandleW() > returns path with trailing '\'. As a result, RtlEqualUnicodeString() > fails and tries to resolve symlink repeatedly. > > For example, RtlEqualUnicodeString() compares \??\UNC\VBoxSrv\tmp and > \??\UNC\VBoxSrv\tmp\, then it fails. > > This does not happen in 32bit Windows10. It seems that UNC path is not > treated as a symlink in 32bit Windows10. I am not sure why. > > Therefore, I have applied a patch which stops to treat UNC path as a > symlink for testing as follows. > > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc > index baf04ce89..31a96ca58 100644 > --- a/winsup/cygwin/path.cc > +++ b/winsup/cygwin/path.cc > @@ -3490,6 +3490,9 @@ restart: > ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0); > if (ret) > { > + if (wcsstr (fpbuf, L"\\\\?\\UNC\\") == fpbuf) > + goto file_not_symlink; > + > UNICODE_STRING fpath; > > RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); > > I have confirmed this patch fixes the issue. In addition, this patch > also resolves the issue: > https://cygwin.com/pipermail/cygwin/2021-December/250103.html > > Is this the right thing? > I have tested the patch and it is now possible to access /cygdrive/z again. But if I create a symlink on that filesystem, it's not identified as a symlink. Although, I don't know if this has ever worked as it is the first time I've ever tested it, it probably hasn't ever worked (see below). user@DESKTOP-******* /cygdrive/z$ ln -s report.pdf test.pdf user@DESKTOP-******* /cygdrive/z$ ls -l report.pdf test.pdf -rw-r--r-- 1 user None 1454562 Nov 28 12:05 report.pdf -rw-r--r-- 1 user None 34 Dec 5 16:36 test.pdf user@DESKTOP-******* /cygdrive/z$ cat test.pdf !<symlink>▒▒report.pdfuser@DESKTOP-******* /cygdrive/z$ I think it's because "special" attributes don't work on VirtualBox shared folders, I can't hide files in Explorer either. So I don't think the patch has caused any regression here. The samba share (Y:) works, just as it did before, symlinks work too. (Sorry about the previous mail, I apparently suck at email.) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-05 14:49 ` Oskar Skog @ 2021-12-06 3:31 ` Takashi Yano 2021-12-06 6:34 ` Oskar Skog 2021-12-06 8:04 ` Takashi Yano 0 siblings, 2 replies; 19+ messages in thread From: Takashi Yano @ 2021-12-06 3:31 UTC (permalink / raw) To: cygwin On Sun, 5 Dec 2021 16:49:13 +0200 Oskar Skog wrote: > But if I create a symlink on that filesystem, it's not identified as a > symlink. Although, I don't know if this has ever worked as it is the > first time I've ever tested it, it probably hasn't ever worked (see > below). > > user@DESKTOP-******* /cygdrive/z$ ln -s report.pdf test.pdf > user@DESKTOP-******* /cygdrive/z$ ls -l report.pdf test.pdf > -rw-r--r-- 1 user None 1454562 Nov 28 12:05 report.pdf > -rw-r--r-- 1 user None 34 Dec 5 16:36 test.pdf > user@DESKTOP-******* /cygdrive/z$ cat test.pdf > !<symlink>▒▒report.pdfuser@DESKTOP-******* /cygdrive/z$ > > I think it's because "special" attributes don't work on VirtualBox > shared folders, I can't hide files in Explorer either. > > So I don't think the patch has caused any regression here. I cannot reproduce that behaviour. yano@DESKTOP-LSNFFD0 /cygdrive/z $ echo AAAAAAAAAAAA > a yano@DESKTOP-LSNFFD0 /cygdrive/z $ ln -s a b yano@DESKTOP-LSNFFD0 /cygdrive/z $ ls -l b lrwxrwxrwx 1 yano None 1 Dec 6 12:17 b -> a yano@DESKTOP-LSNFFD0 /cygdrive/z $ cat b AAAAAAAAAAAA yano@DESKTOP-LSNFFD0 /cygdrive/z $ mount C:/cygwin/bin on /usr/bin type ntfs (binary,auto) C:/cygwin/lib on /usr/lib type ntfs (binary,auto) C:/cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) Are you running VirtualBox in non-windows host by any chance? cygwin symbolic link seems to work only in NTFS file system. If shared folder is in linux file system, cygwin symbolic link does not work. -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-06 3:31 ` Takashi Yano @ 2021-12-06 6:34 ` Oskar Skog 2021-12-06 8:04 ` Takashi Yano 1 sibling, 0 replies; 19+ messages in thread From: Oskar Skog @ 2021-12-06 6:34 UTC (permalink / raw) To: cygwin On 2021-12-06 05:31, Takashi Yano wrote: > On Sun, 5 Dec 2021 16:49:13 +0200 > Oskar Skog wrote: >> But if I create a symlink on that filesystem, it's not identified as a >> symlink. Although, I don't know if this has ever worked as it is the >> first time I've ever tested it, it probably hasn't ever worked (see >> below). >> >> user@DESKTOP-******* /cygdrive/z$ ln -s report.pdf test.pdf >> user@DESKTOP-******* /cygdrive/z$ ls -l report.pdf test.pdf >> -rw-r--r-- 1 user None 1454562 Nov 28 12:05 report.pdf >> -rw-r--r-- 1 user None 34 Dec 5 16:36 test.pdf >> user@DESKTOP-******* /cygdrive/z$ cat test.pdf >> !<symlink>▒▒report.pdfuser@DESKTOP-******* /cygdrive/z$ >> >> I think it's because "special" attributes don't work on VirtualBox >> shared folders, I can't hide files in Explorer either. >> >> So I don't think the patch has caused any regression here. > > I cannot reproduce that behaviour. > > yano@DESKTOP-LSNFFD0 /cygdrive/z > $ echo AAAAAAAAAAAA > a > > yano@DESKTOP-LSNFFD0 /cygdrive/z > $ ln -s a b > > yano@DESKTOP-LSNFFD0 /cygdrive/z > $ ls -l b > lrwxrwxrwx 1 yano None 1 Dec 6 12:17 b -> a > > yano@DESKTOP-LSNFFD0 /cygdrive/z > $ cat b > AAAAAAAAAAAA > > yano@DESKTOP-LSNFFD0 /cygdrive/z > $ mount > C:/cygwin/bin on /usr/bin type ntfs (binary,auto) > C:/cygwin/lib on /usr/lib type ntfs (binary,auto) > C:/cygwin on / type ntfs (binary,auto) > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) > > > Are you running VirtualBox in non-windows host by any chance? > cygwin symbolic link seems to work only in NTFS file system. > > If shared folder is in linux file system, cygwin symbolic link > does not work. > Yes, I'm on a Linux host. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-06 3:31 ` Takashi Yano 2021-12-06 6:34 ` Oskar Skog @ 2021-12-06 8:04 ` Takashi Yano 1 sibling, 0 replies; 19+ messages in thread From: Takashi Yano @ 2021-12-06 8:04 UTC (permalink / raw) To: cygwin On Mon, 6 Dec 2021 12:31:35 +0900 Takashi Yano wrote: > cygwin symbolic link seems to work only in NTFS file system. This is not right. cygwin symlink needs 'SYSTEM' attribute. So it does not work in file system which does not support that attributes. Windows file system, such as NTFS, FAT, supports it. > If shared folder is in linux file system, cygwin symbolic link > does not work. Basically, linux file system does not support 'SYSTEM' attribute, but samba support it using extended attributes if 'store dos attributes' is set to 'yes' (by default since samba 4.9). -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-05 2:54 ` Takashi Yano 2021-12-05 14:49 ` Oskar Skog @ 2021-12-06 10:16 ` Corinna Vinschen 2021-12-06 10:55 ` Takashi Yano 1 sibling, 1 reply; 19+ messages in thread From: Corinna Vinschen @ 2021-12-06 10:16 UTC (permalink / raw) To: cygwin On Dec 5 11:54, Takashi Yano wrote: > On Tue, 30 Nov 2021 19:04:57 +0200 > Oskar Skog wrote: > > vboxsharedfs file systems no longer work. Any attempt to access will > > result in "too many levels of symbolic links". > > > > This only affects the VirtualBox shared folder, the Samba share works > > just fine. > > > > Last time I updated (before today) was sometime before the 10th of > > September. > > > > MSYS2 has the same problem, but no one seems to have reported it to > > Cygwin: > > https://github.com/msys2/msys2-runtime/issues/58 > > > > > > user@DESKTOP-******* ~$ ls /cygdrive/z > > ls: cannot access '/cygdrive/z': Too many levels of symbolic links > > user@DESKTOP-******* ~$ mount > > C:/Cygwin/bin on /usr/bin type ntfs (binary,auto) > > C:/Cygwin/lib on /usr/lib type ntfs (binary,auto) > > C:/Cygwin on / type ntfs (binary,auto) > > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > > Y: on /cygdrive/y type smbfs (binary,posix=0,user,noumount,auto) > > Z: on /cygdrive/z type vboxsharedfolderfs > > (binary,posix=0,user,noumount,auto) > > user@DESKTOP-******* ~$ uname -a > > CYGWIN_NT-10.0 DESKTOP-* 3.3.2(0.341/5/3) 2021-11-08 16:55 x86_64 Cygwin > > Thanks for the report. > It seems that this happens only in 64bit Windoes10/11. > [...] > I tested with VirtualBox version 6.1.30. Thanks for testing, Takashi! > In 64bit Windows10, for vbox shared path, GetFinalPathNameByHandleW() > returns path with trailing '\'. As a result, RtlEqualUnicodeString() > fails and tries to resolve symlink repeatedly. That sounds like a bug in VirtualBox, but yeah, we should workaround it. > For example, RtlEqualUnicodeString() compares \??\UNC\VBoxSrv\tmp and > \??\UNC\VBoxSrv\tmp\, then it fails. > [...] > + if (wcsstr (fpbuf, L"\\\\?\\UNC\\") == fpbuf) > + goto file_not_symlink; > + Isn't that a bit intrusive? Wouldn't it also fix the issue if we just overwrite a trailing '\\' with '\0', like this? diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index baf04ce89a08..b76e5b0466cf 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -3492,8 +3492,14 @@ restart: { UNICODE_STRING fpath; - RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); + /* If incoming path has no trailing backslash, but final path + has one, drop trailing backslash from final path so the + below string comparison has a chance to succeed. */ + if (upath.Buffer[(upath.Length - 1) / sizeof (WCHAR)] != L'\\' + && fpbuf[ret - 1] == L'\\') + fpbuf[--ret] = L'\0'; fpbuf[1] = L'?'; /* \\?\ --> \??\ */ + RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); if (!RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) { issymlink = true; Thanks, Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-06 10:16 ` Corinna Vinschen @ 2021-12-06 10:55 ` Takashi Yano 2021-12-07 0:46 ` Takashi Yano 0 siblings, 1 reply; 19+ messages in thread From: Takashi Yano @ 2021-12-06 10:55 UTC (permalink / raw) To: cygwin On Mon, 6 Dec 2021 11:16:30 +0100 Corinna Vinschen wrote: > > For example, RtlEqualUnicodeString() compares \??\UNC\VBoxSrv\tmp and > > \??\UNC\VBoxSrv\tmp\, then it fails. > > [...] > > + if (wcsstr (fpbuf, L"\\\\?\\UNC\\") == fpbuf) > > + goto file_not_symlink; > > + > > Isn't that a bit intrusive? Wouldn't it also fix the issue if we > just overwrite a trailing '\\' with '\0', like this? > > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc > index baf04ce89a08..b76e5b0466cf 100644 > --- a/winsup/cygwin/path.cc > +++ b/winsup/cygwin/path.cc > @@ -3492,8 +3492,14 @@ restart: > { > UNICODE_STRING fpath; > > - RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); > + /* If incoming path has no trailing backslash, but final path > + has one, drop trailing backslash from final path so the > + below string comparison has a chance to succeed. */ > + if (upath.Buffer[(upath.Length - 1) / sizeof (WCHAR)] != L'\\' > + && fpbuf[ret - 1] == L'\\') > + fpbuf[--ret] = L'\0'; > fpbuf[1] = L'?'; /* \\?\ --> \??\ */ > + RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); > if (!RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) > { > issymlink = true; First, I think the same. However, with this patch, it sometimes causes hang for a few seconds around the code: else if (isvirtual_dev (dev)) { /* FIXME: Calling build_fhandler here is not the right way to handle this. */ fhandler_virtual *fh = (fhandler_virtual *) build_fh_dev (dev, path_copy); virtual_ftype_t file_type; if (!fh) file_type = virt_none; else { file_type = fh->exists (); if (file_type == virt_symlink || file_type == virt_fdsymlink) { fh->fill_filebuf (); symlen = sym.set (fh->get_filebuf ()); } with path_copy of "//VBoxSrv". I am not sure why. In addition, https://cygwin.com/pipermail/cygwin/2021-December/250103.html still needs fix. So I proposed the patch which stops to treat UNC path as symlink. -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-06 10:55 ` Takashi Yano @ 2021-12-07 0:46 ` Takashi Yano 2021-12-07 14:57 ` Corinna Vinschen 0 siblings, 1 reply; 19+ messages in thread From: Takashi Yano @ 2021-12-07 0:46 UTC (permalink / raw) To: cygwin On Mon, 6 Dec 2021 19:55:27 +0900 Takashi Yano wrote: > First, I think the same. However, with this patch, it sometimes causes > hang for a few seconds around the code: [...] > with path_copy of "//VBoxSrv". I am not sure why. > > In addition, > https://cygwin.com/pipermail/cygwin/2021-December/250103.html > still needs fix. Moreover, this has a problem with "ls -al /cygdrive/z". The information for ".." is not retrieved correctly. drwxr-xr-x 1 yano None 0 Dec 6 13:54 . d????????? ? ? ? ? ? .. -rw-r--r-- 1 yano None 29 Dec 4 07:45 Makefile -rw-r--r-- 1 yano None 17 Dec 6 13:18 a drwxr-xr-x 1 yano None 0 Dec 6 08:59 aaa lrwxrwxrwx 1 yano None 1 Dec 6 13:54 b -> a -rwxr-xr-x 1 yano None 3278626 Dec 7 09:07 cygwin0.dll -rwxr-xr-x 1 yano None 3549860 Dec 6 08:51 cygwin0.dll.64 -rw-r--r-- 1 yano None 0 Dec 3 22:16 testfile.txt I think '/cygdrive/z/..' should be '/cygdrive', however, in current cygwin, it is interpreted into '//VBoxSrv'. Is this as you intended? With my patch which stops to treat UNC path as symlink, '/cygdrive/z/..' returns '/cygdrive'. -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-07 0:46 ` Takashi Yano @ 2021-12-07 14:57 ` Corinna Vinschen 2021-12-07 15:32 ` Takashi Yano 0 siblings, 1 reply; 19+ messages in thread From: Corinna Vinschen @ 2021-12-07 14:57 UTC (permalink / raw) To: cygwin On Dec 7 09:46, Takashi Yano wrote: > On Mon, 6 Dec 2021 19:55:27 +0900 > Takashi Yano wrote: > > First, I think the same. However, with this patch, it sometimes causes > > hang for a few seconds around the code: > [...] > > with path_copy of "//VBoxSrv". I am not sure why. That's expected and nothing really to worry about. It's the network environment access in fhandler_netdrive. <sidenote> I have this on the radar already, because the WNet stuff in there has been broken by Microsoft with certain updates to Windows 10. A SMB security fix has deliberately left behind the WNet calls to enumerate the network environment, because they rely on functionality of older SMB versions. </sidenote> > > > > In addition, > > https://cygwin.com/pipermail/cygwin/2021-December/250103.html > > still needs fix. > > Moreover, this has a problem with "ls -al /cygdrive/z". > The information for ".." is not retrieved correctly. > > drwxr-xr-x 1 yano None 0 Dec 6 13:54 . > d????????? ? ? ? ? ? .. > -rw-r--r-- 1 yano None 29 Dec 4 07:45 Makefile > -rw-r--r-- 1 yano None 17 Dec 6 13:18 a > drwxr-xr-x 1 yano None 0 Dec 6 08:59 aaa > lrwxrwxrwx 1 yano None 1 Dec 6 13:54 b -> a > -rwxr-xr-x 1 yano None 3278626 Dec 7 09:07 cygwin0.dll > -rwxr-xr-x 1 yano None 3549860 Dec 6 08:51 cygwin0.dll.64 > -rw-r--r-- 1 yano None 0 Dec 3 22:16 testfile.txt > > I think '/cygdrive/z/..' should be '/cygdrive', however, > in current cygwin, it is interpreted into '//VBoxSrv'. > > Is this as you intended? > > With my patch which stops to treat UNC path as symlink, > '/cygdrive/z/..' returns '/cygdrive'. Yeah, but... ...what bugs me is that *every* UNC path is treated this way with that patch. If you have a path like /cygdrive/x/a/b/c, with x: being a virtual drive pointing to //server/share, and with "b" being a symlink to "syml", what you get back is either //server/share/a/syml/c without, or /cygdrive/x/a/b/c with your patch. What we would like to get back is /cygdrive/x/a/syml/c So the real problem is not that we have an UNC path, but the fact that the drive letter expression is (correctly, but unwanted) converted to the matching UNC path by GetFinalPathNameByHandle. Bottom line is, your patch is ok, please apply. It would be nice, though, if we could just avoid the drive: -> UNC path conversion and keep the rest. Thanks, Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-07 14:57 ` Corinna Vinschen @ 2021-12-07 15:32 ` Takashi Yano 2021-12-07 15:43 ` Takashi Yano 2021-12-07 16:03 ` Corinna Vinschen 0 siblings, 2 replies; 19+ messages in thread From: Takashi Yano @ 2021-12-07 15:32 UTC (permalink / raw) To: cygwin On Tue, 7 Dec 2021 15:57:56 +0100 Corinna Vinschen wrote: > On Dec 7 09:46, Takashi Yano wrote: > > I think '/cygdrive/z/..' should be '/cygdrive', however, > > in current cygwin, it is interpreted into '//VBoxSrv'. > > > > Is this as you intended? > > > > With my patch which stops to treat UNC path as symlink, > > '/cygdrive/z/..' returns '/cygdrive'. > > Yeah, but... > > ...what bugs me is that *every* UNC path is treated this way with > that patch. If you have a path like /cygdrive/x/a/b/c, with x: > being a virtual drive pointing to //server/share, and with "b" > being a symlink to "syml", what you get back is either > > //server/share/a/syml/c > > without, or > > /cygdrive/x/a/b/c > > with your patch. What we would like to get back is > > /cygdrive/x/a/syml/c With my patch, above case behaves like: $ mount C:/cygwin/bin on /usr/bin type ntfs (binary,auto) C:/cygwin/lib on /usr/lib type ntfs (binary,auto) C:/cygwin on / type ntfs (binary,auto) C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) $ cd /cygdrive/z $ mkdir -p aa/syml/cc $ ln -s syml aa/bb $ cd aa/bb/cc $ /bin/pwd /cygdrive/z/aa/syml/cc $ Isn't this what you would like? > So the real problem is not that we have an UNC path, but the fact that > the drive letter expression is (correctly, but unwanted) converted to > the matching UNC path by GetFinalPathNameByHandle. > > Bottom line is, your patch is ok, please apply. It would be nice, > though, if we could just avoid the drive: -> UNC path conversion and > keep the rest. -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-07 15:32 ` Takashi Yano @ 2021-12-07 15:43 ` Takashi Yano 2021-12-07 16:03 ` Corinna Vinschen 1 sibling, 0 replies; 19+ messages in thread From: Takashi Yano @ 2021-12-07 15:43 UTC (permalink / raw) To: cygwin On Wed, 8 Dec 2021 00:32:49 +0900 Takashi Yano wrote: > With my patch, above case behaves like: > > $ mount > C:/cygwin/bin on /usr/bin type ntfs (binary,auto) > C:/cygwin/lib on /usr/lib type ntfs (binary,auto) > C:/cygwin on / type ntfs (binary,auto) > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) > $ cd /cygdrive/z > $ mkdir -p aa/syml/cc > $ ln -s syml aa/bb > $ cd aa/bb/cc > $ /bin/pwd > /cygdrive/z/aa/syml/cc > $ $ cygpath -a . /cygdrive/z/aa/syml/cc/ $ echo -e 'all:\n\t@echo CURDIR=$(CURDIR)' > Makefile $ make -C . make: Entering directory '/cygdrive/z/aa/syml/cc' CURDIR=/cygdrive/z/aa/syml/cc make: Leaving directory '/cygdrive/z/aa/syml/cc' > Isn't this what you would like? -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-07 15:32 ` Takashi Yano 2021-12-07 15:43 ` Takashi Yano @ 2021-12-07 16:03 ` Corinna Vinschen 1 sibling, 0 replies; 19+ messages in thread From: Corinna Vinschen @ 2021-12-07 16:03 UTC (permalink / raw) To: cygwin On Dec 8 00:32, Takashi Yano wrote: > On Tue, 7 Dec 2021 15:57:56 +0100 > Corinna Vinschen wrote: > > On Dec 7 09:46, Takashi Yano wrote: > > > I think '/cygdrive/z/..' should be '/cygdrive', however, > > > in current cygwin, it is interpreted into '//VBoxSrv'. > > > > > > Is this as you intended? > > > > > > With my patch which stops to treat UNC path as symlink, > > > '/cygdrive/z/..' returns '/cygdrive'. > > > > Yeah, but... > > > > ...what bugs me is that *every* UNC path is treated this way with > > that patch. If you have a path like /cygdrive/x/a/b/c, with x: > > being a virtual drive pointing to //server/share, and with "b" > > being a symlink to "syml", what you get back is either > > > > //server/share/a/syml/c > > > > without, or > > > > /cygdrive/x/a/b/c > > > > with your patch. What we would like to get back is > > > > /cygdrive/x/a/syml/c > > With my patch, above case behaves like: > > $ mount > C:/cygwin/bin on /usr/bin type ntfs (binary,auto) > C:/cygwin/lib on /usr/lib type ntfs (binary,auto) > C:/cygwin on / type ntfs (binary,auto) > C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto) > D: on /cygdrive/d type iso9660 (binary,posix=0,user,noumount,auto) > Z: on /cygdrive/z type vboxsharedfolderfs (binary,posix=0,user,noumount,auto) > $ cd /cygdrive/z > $ mkdir -p aa/syml/cc > $ ln -s syml aa/bb > $ cd aa/bb/cc > $ /bin/pwd > /cygdrive/z/aa/syml/cc > $ > > Isn't this what you would like? Sorry, I wasn't expressing myself clearly. The end result is what is expected, yes. But that's the result of the full path_conv conversion, which wasn't what I was up to. The idea of the GFPNBH call is to short-circuit the path_conv handling in case we have native Windows symlinks in the path. My example above was only considering what comes out of the `if ((pc_flags & ...) { ... } ' expression starting in line 3485 (assuming "b" is a native symlink). What I mean is this: Your patch disregards the entire string returned by GFPNBH, if the returned path is an UNC path, no matter what. But what if the incoming path already *was* an UNC path, and potentially contains native symlinks? In that case you have no reason to disregard the resulting path from GFPNBH. And if it was a drive letter path, wouldn't it be nicer to just convert the UNC path prefix back to the drive letter and keep the rest of the final path intact? However, both of these scenarios require extra code, which isn't that important for now, so, never mind. Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <Ya+WvmECA+zAbuV9@calimero.vinschen.de>]
* Re: vboxsharedfs - Too many levels of symbolic links [not found] <Ya+WvmECA+zAbuV9@calimero.vinschen.de> @ 2021-12-08 8:20 ` Takashi Yano 2021-12-08 10:37 ` Corinna Vinschen 0 siblings, 1 reply; 19+ messages in thread From: Takashi Yano @ 2021-12-08 8:20 UTC (permalink / raw) To: cygwin [-- Attachment #1: Type: text/plain, Size: 4113 bytes --] On Tue, 7 Dec 2021 18:15:42 +0100 Corinna Vinschen wrote: > Hi Takashi, > > ----- Forwarded message from Corinna Vinschen <corinna-cygwin@cygwin.com> ----- > > The idea of the GFPNBH call is to short-circuit the path_conv handling > > in case we have native Windows symlinks in the path. My example above > > was only considering what comes out of the `if ((pc_flags & ...) { ... } > > ' expression starting in line 3485 (assuming "b" is a native symlink). > > > > What I mean is this: Your patch disregards the entire string returned by > > GFPNBH, if the returned path is an UNC path, no matter what. > > > > But what if the incoming path already *was* an UNC path, and potentially > > contains native symlinks? In that case you have no reason to disregard > > the resulting path from GFPNBH. > > > > And if it was a drive letter path, wouldn't it be nicer to just convert > > the UNC path prefix back to the drive letter and keep the rest of the > > final path intact? > > > > However, both of these scenarios require extra code, which isn't that > > important for now, so, never mind. > ----- End forwarded message ----- > > What I meant is something like the below (entirely untested). Yeah, I'm > not sure myself, if it's worth the effort... > > diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc > index e254397257fd..06afd2d62996 100644 > --- a/winsup/cygwin/autoload.cc > +++ b/winsup/cygwin/autoload.cc > @@ -630,6 +630,7 @@ LoadDLLfunc (LdapMapErrorToWin32, 0, wldap32) > > LoadDLLfunc (WNetCloseEnum, 4, mpr) > LoadDLLfunc (WNetEnumResourceW, 16, mpr) > +LoadDLLfunc (WNetGetConnectionW, 12, mpr) > LoadDLLfunc (WNetGetProviderNameW, 12, mpr) > LoadDLLfunc (WNetGetResourceInformationW, 16, mpr) > LoadDLLfunc (WNetOpenEnumW, 20, mpr) > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc > index baf04ce89a08..c7b29acf29b3 100644 > --- a/winsup/cygwin/path.cc > +++ b/winsup/cygwin/path.cc > @@ -3492,10 +3492,41 @@ restart: > { > UNICODE_STRING fpath; > > - RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); > + /* If incoming path has no trailing backslash, but final path > + has one, drop trailing backslash from final path so the > + below string comparison has a chance to succeed. */ > + if (upath.Buffer[(upath.Length - 1) / sizeof (WCHAR)] != L'\\' > + && fpbuf[ret - 1] == L'\\') > + fpbuf[--ret] = L'\0'; > fpbuf[1] = L'?'; /* \\?\ --> \??\ */ > + RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); > if (!RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) > { > + /* Check if the final path is an UNC path and the incoming > + path isn't. If so... */ > + if (RtlEqualUnicodePathPrefix (&fpath, &ro_u_uncp, TRUE) > + && !RtlEqualUnicodePathPrefix (&upath, &ro_u_uncp, TRUE)) > + { > + /* ...extract the drive letter, get the remote path, > + replace remote path with drive letter, check again. */ > + WCHAR drive[3] = { upath.Buffer[4], L':', L'\0' }; > + WCHAR remote[MAX_PATH]; > + > + if (WNetGetConnectionW (drive, remote, MAX_PATH) > + == NO_ERROR) > + { > + /* Hackfest */ > + fpath.Buffer[4] = drive[0]; > + fpath.Buffer[5] = L':'; > + WCHAR to = fpath.Buffer + 6; > + WCHAR from = to + wcslen (remote); > + memmove (to, from, > + (wcslen (from) + 1) * sizeof (WCHAR)); > + fpath.Length -= (from - to) * sizeof (WCHAR); > + if (RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) > + goto file_not_symlink; > + } > + } > issymlink = true; > /* upath.Buffer is big enough and unused from this point on. > Reuse it here, avoiding yet another buffer allocation. */ I confirmed that your patch works nicely. ...except when the drive is created by subst using UNC path, e.g. subst w: \\server\share. In this case, WNetGetConnectionW() fails with ERROR_NO_NET_OR_BAD_PATH. So, I modified your patch a bit. What about attached patch? -- Takashi Yano <takashi.yano@nifty.ne.jp> [-- Attachment #2: 0001-Cygwin-path-Convert-UNC-path-prefix-back-to-drive-le.patch --] [-- Type: application/octet-stream, Size: 2766 bytes --] From dc222ec73f537e9e47558bbf156dd1735b9095e5 Mon Sep 17 00:00:00 2001 From: Takashi Yano <takashi.yano@nifty.ne.jp> Date: Wed, 8 Dec 2021 17:09:14 +0900 Subject: [PATCH] Cygwin: path: Convert UNC path prefix back to drive letter. - When symlink_info::check() is called with the path having drive letter and UNC path is mounted to the drive, the path is replaced with UNC path. With this patch, UNC path prefix is converted back to drive letter. This fixes the issue: https://cygwin.com/pipermail/cygwin/2021-November/250087.html https://cygwin.com/pipermail/cygwin/2021-December/250103.html --- winsup/cygwin/path.cc | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index baf04ce89..eb1255849 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -3492,10 +3492,45 @@ restart: { UNICODE_STRING fpath; - RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); + /* If incoming path has no trailing backslash, but final path + has one, drop trailing backslash from final path so the + below string comparison has a chance to succeed. */ + if (upath.Buffer[(upath.Length - 1) / sizeof (WCHAR)] != L'\\' + && fpbuf[ret - 1] == L'\\') + fpbuf[--ret] = L'\0'; fpbuf[1] = L'?'; /* \\?\ --> \??\ */ + RtlInitCountedUnicodeString (&fpath, fpbuf, ret * sizeof (WCHAR)); if (!RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) { + /* Check if the final path is an UNC path and the incoming + path isn't. If so... */ + if (RtlEqualUnicodePathPrefix (&fpath, &ro_u_uncp, TRUE) + && !RtlEqualUnicodePathPrefix (&upath, &ro_u_uncp, TRUE)) + { + /* ...get the remote path from the volume path name, + replace remote path with drive letter, check again. */ + WCHAR remote[MAX_PATH]; + + fpbuf[1] = L'\\'; + BOOL r = GetVolumePathNameW (fpbuf, remote, MAX_PATH); + fpbuf[1] = L'?'; + if (r) + { + int remlen = wcslen (remote); + if (remote[remlen - 1] == L'\\') + remlen--; + /* Hackfest */ + fpath.Buffer[4] = upath.Buffer[4]; /* Drive letter */ + fpath.Buffer[5] = L':'; + WCHAR *to = fpath.Buffer + 6; + WCHAR *from = to + remlen - 6; + memmove (to, from, + (wcslen (from) + 1) * sizeof (WCHAR)); + fpath.Length -= (from - to) * sizeof (WCHAR); + if (RtlEqualUnicodeString (&upath, &fpath, !!ci_flag)) + goto file_not_symlink; + } + } issymlink = true; /* upath.Buffer is big enough and unused from this point on. Reuse it here, avoiding yet another buffer allocation. */ -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-08 8:20 ` Takashi Yano @ 2021-12-08 10:37 ` Corinna Vinschen 2021-12-09 8:16 ` Takashi Yano 0 siblings, 1 reply; 19+ messages in thread From: Corinna Vinschen @ 2021-12-08 10:37 UTC (permalink / raw) To: cygwin On Dec 8 17:20, Takashi Yano wrote: > On Tue, 7 Dec 2021 18:15:42 +0100 > Corinna Vinschen wrote: > > Hi Takashi, > > > > ----- Forwarded message from Corinna Vinschen <corinna-cygwin@cygwin.com> ----- > > > The idea of the GFPNBH call is to short-circuit the path_conv handling > > > in case we have native Windows symlinks in the path. My example above > > > was only considering what comes out of the `if ((pc_flags & ...) { ... } > > > ' expression starting in line 3485 (assuming "b" is a native symlink). > > > > > > What I mean is this: Your patch disregards the entire string returned by > > > GFPNBH, if the returned path is an UNC path, no matter what. > > > > > > But what if the incoming path already *was* an UNC path, and potentially > > > contains native symlinks? In that case you have no reason to disregard > > > the resulting path from GFPNBH. > > > > > > And if it was a drive letter path, wouldn't it be nicer to just convert > > > the UNC path prefix back to the drive letter and keep the rest of the > > > final path intact? > > > > > > However, both of these scenarios require extra code, which isn't that > > > important for now, so, never mind. > > ----- End forwarded message ----- > > > > What I meant is something like the below (entirely untested). Yeah, I'm > > not sure myself, if it's worth the effort... > > [...] > I confirmed that your patch works nicely. > > ...except when the drive is created by subst using UNC path, > e.g. subst w: \\server\share. > > In this case, WNetGetConnectionW() fails with ERROR_NO_NET_OR_BAD_PATH. > > So, I modified your patch a bit. > > What about attached patch? Oh, great! GetVolumePathNameW is the function I somehow missed when looking into the Microsoft docs yesterday, so thanks for modifying the patch this way. Please push. Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-08 10:37 ` Corinna Vinschen @ 2021-12-09 8:16 ` Takashi Yano 2021-12-09 9:16 ` Corinna Vinschen 0 siblings, 1 reply; 19+ messages in thread From: Takashi Yano @ 2021-12-09 8:16 UTC (permalink / raw) To: cygwin On Wed, 8 Dec 2021 11:37:39 +0100 Corinna Vinschen wrote: > On Dec 8 17:20, Takashi Yano wrote: > > On Tue, 7 Dec 2021 18:15:42 +0100 > > I confirmed that your patch works nicely. > > > > ...except when the drive is created by subst using UNC path, > > e.g. subst w: \\server\share. > > > > In this case, WNetGetConnectionW() fails with ERROR_NO_NET_OR_BAD_PATH. > > > > So, I modified your patch a bit. > > > > What about attached patch? > > Oh, great! GetVolumePathNameW is the function I somehow missed when > looking into the Microsoft docs yesterday, so thanks for modifying the > patch this way. > > Please push. I noticed that this code does not work if UNC path "\\server\share\dir" (rather than "\\server\share") is mounted to virtual drive. I will submit a patch for this issue. Sorry for testing insufficiently. -- Takashi Yano <takashi.yano@nifty.ne.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: vboxsharedfs - Too many levels of symbolic links 2021-12-09 8:16 ` Takashi Yano @ 2021-12-09 9:16 ` Corinna Vinschen 0 siblings, 0 replies; 19+ messages in thread From: Corinna Vinschen @ 2021-12-09 9:16 UTC (permalink / raw) To: cygwin On Dec 9 17:16, Takashi Yano wrote: > On Wed, 8 Dec 2021 11:37:39 +0100 > Corinna Vinschen wrote: > > On Dec 8 17:20, Takashi Yano wrote: > > > On Tue, 7 Dec 2021 18:15:42 +0100 > > > I confirmed that your patch works nicely. > > > > > > ...except when the drive is created by subst using UNC path, > > > e.g. subst w: \\server\share. > > > > > > In this case, WNetGetConnectionW() fails with ERROR_NO_NET_OR_BAD_PATH. > > > > > > So, I modified your patch a bit. > > > > > > What about attached patch? > > > > Oh, great! GetVolumePathNameW is the function I somehow missed when > > looking into the Microsoft docs yesterday, so thanks for modifying the > > patch this way. > > > > Please push. > > I noticed that this code does not work if UNC path "\\server\share\dir" > (rather than "\\server\share") is mounted to virtual drive. Oh drat, I tried that explicitely and I saw that WNetGetConnectionW returns the same as `net use' on the command line. So GetVolumePathNameW only returns the actual share name? Sigh. > I will submit a patch for this issue. Sorry for testing insufficiently. No worries and thanks, Corinna ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-12-09 9:16 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-30 17:04 vboxsharedfs - Too many levels of symbolic links Oskar Skog 2021-11-30 21:40 ` Gackle, Philip P 2021-12-02 10:43 ` Corinna Vinschen 2021-12-05 2:54 ` Takashi Yano 2021-12-05 14:49 ` Oskar Skog 2021-12-06 3:31 ` Takashi Yano 2021-12-06 6:34 ` Oskar Skog 2021-12-06 8:04 ` Takashi Yano 2021-12-06 10:16 ` Corinna Vinschen 2021-12-06 10:55 ` Takashi Yano 2021-12-07 0:46 ` Takashi Yano 2021-12-07 14:57 ` Corinna Vinschen 2021-12-07 15:32 ` Takashi Yano 2021-12-07 15:43 ` Takashi Yano 2021-12-07 16:03 ` Corinna Vinschen [not found] <Ya+WvmECA+zAbuV9@calimero.vinschen.de> 2021-12-08 8:20 ` Takashi Yano 2021-12-08 10:37 ` Corinna Vinschen 2021-12-09 8:16 ` Takashi Yano 2021-12-09 9:16 ` Corinna Vinschen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).