vboxsharedfs - Too many levels of symbolic links

[Takashi Yano]

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>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cygwin-path-Convert-UNC-path-prefix-back-to-drive-le.patch
Type: application/octet-stream
Size: 2766 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin/attachments/20211208/0a602f91/attachment.obj>