public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* 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 ` vboxsharedfs - Too many levels of symbolic links 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

* 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

* 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 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  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-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-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-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  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-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-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-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-11-30 17:04 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-11-30 17:04 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 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&amp;data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765066642%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=FQ%2FLAAfIRvLXr7gL1iqVQkspgWXL2WY59FB8fvx8DqI%3D&amp;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&amp;data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=OUOovyeBCPL1b6mSQOHHI3k0GBQEETJV71wPfhYUltU%3D&amp;reserved=0
FAQ:                  https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Ffaq%2F&amp;data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=8mQ%2FNyYJGT5J5P3pLOtu1FWQaoPWQIHZgXmfaeeI4WM%3D&amp;reserved=0
Documentation:        https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Fdocs.html&amp;data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=l8lbrBZTWc7wJIsPLKI%2Bvsj4zzW%2Fi9RNFNvXQge%2B%2FCI%3D&amp;reserved=0
Unsubscribe info:     https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcygwin.com%2Fml%2F%23unsubscribe-simple&amp;data=04%7C01%7CPhilip.Gackle%40pnnl.gov%7Cc9ed5aca3b89499b931508d9b423ad0d%7Cd6faa5f90ae240338c0130048a38deeb%7C0%7C0%7C637738888765076597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=wtlkvSXvzCzYKQlO2voUxYgq7DoiQ5Lte08rLrAw9m4%3D&amp;reserved=0

^ permalink raw reply	[flat|nested] 19+ messages in thread

* 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

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 --
     [not found] <Ya+WvmECA+zAbuV9@calimero.vinschen.de>
2021-12-08  8:20 ` vboxsharedfs - Too many levels of symbolic links Takashi Yano
2021-12-08 10:37   ` Corinna Vinschen
2021-12-09  8:16     ` Takashi Yano
2021-12-09  9:16       ` Corinna Vinschen
2021-11-30 17:04 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

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).