Root directory being a junction fools realpath()

Root directory being a junction fools realpath()

[David Macek]

Greetings.

I think I found a way to fool Cygwin into misplacing its root
directory. Given recent Cygwin with GCC and a simple test program:

```
#include <stdio.h>
#include <limits.h>
#include <stdlib.h>
int main() {
char real[PATH_MAX];
realpath("/", real);
printf("/ -> %s\n", real);
return 0;
}
```

... compiled with:

$ gcc -D_DEFAULT_SOURCE -o new.exe -Wall -Wextra -std=c11 new.c

... and my set-up with a junction (`mklink /j link target`) realpath()
result is dependent on which path I use to invoke the Cygwin root
process:

> dir C:\ | findstr cygwin64
10. 02. 2021  12:13    <DIR>          cygwin64

> dir D:\ | findstr cygwin64
14. 02. 2021  12:19    <JUNCTION>     cygwin64 [C:\cygwin64]

> C:\cygwin64\bin\bash -lc /cygdrive/w/new
/ -> /

> D:\cygwin64\bin\bash -lc /cygdrive/w/new
/ -> /cygdrive/c/cygwin64

It seems the current working directory doesn't matter at all, same as
the directory where the test program resides. Regular substs don't
seem to have the same effect. I can do more tests if requested.

I assume this is not intended behavior, because the root should always
be the root for Cygwin processes, no matter how they're invoked.

If possible ping me on IRC if anything because I don't check my mail too often.

-- 
David Macek

Re: Root directory being a junction fools realpath()

[Corinna Vinschen]

On Feb 14 12:56, David Macek via Cygwin-developers wrote:
> Greetings.
> 
> I think I found a way to fool Cygwin into misplacing its root
> directory. Given recent Cygwin with GCC and a simple test program:
> 
> ```
> #include <stdio.h>
> #include <limits.h>
> #include <stdlib.h>
> int main() {
> char real[PATH_MAX];
> realpath("/", real);
> printf("/ -> %s\n", real);
> return 0;
> }
> ```
> 
> ... compiled with:
> 
> $ gcc -D_DEFAULT_SOURCE -o new.exe -Wall -Wextra -std=c11 new.c
> 
> ... and my set-up with a junction (`mklink /j link target`) realpath()
> result is dependent on which path I use to invoke the Cygwin root
> process:
> 
> > dir C:\ | findstr cygwin64
> 10. 02. 2021  12:13    <DIR>          cygwin64
> 
> > dir D:\ | findstr cygwin64
> 14. 02. 2021  12:19    <JUNCTION>     cygwin64 [C:\cygwin64]
> 
> > C:\cygwin64\bin\bash -lc /cygdrive/w/new
> / -> /
> 
> > D:\cygwin64\bin\bash -lc /cygdrive/w/new
> / -> /cygdrive/c/cygwin64

This isn't realpath's fault.  You're circumventing the mount point
handling which is automated in terms of /, depending on the path
returned from GetModuleFileNameW for the Cygwin DLL.  Since you're
calling D:\cygwin64\bin\bash the dir returned from GetModuleFileNameW is
D:\cygwin64\bin, thus root is D:\cygwin64.

However, junctions are treated as symlinks in Cygwin.  Thus the result
you see above.


Corinna

Re: Root directory being a junction fools realpath()

[Corinna Vinschen]

On Feb 15 13:25, Corinna Vinschen via Cygwin-developers wrote:
> On Feb 14 12:56, David Macek via Cygwin-developers wrote:
> > Greetings.
> > 
> > I think I found a way to fool Cygwin into misplacing its root
> > directory. Given recent Cygwin with GCC and a simple test program:
> > 
> > ```
> > #include <stdio.h>
> > #include <limits.h>
> > #include <stdlib.h>
> > int main() {
> > char real[PATH_MAX];
> > realpath("/", real);
> > printf("/ -> %s\n", real);
> > return 0;
> > }
> > ```
> > 
> > ... compiled with:
> > 
> > $ gcc -D_DEFAULT_SOURCE -o new.exe -Wall -Wextra -std=c11 new.c
> > 
> > ... and my set-up with a junction (`mklink /j link target`) realpath()
> > result is dependent on which path I use to invoke the Cygwin root
> > process:
> > 
> > > dir C:\ | findstr cygwin64
> > 10. 02. 2021  12:13    <DIR>          cygwin64
> > 
> > > dir D:\ | findstr cygwin64
> > 14. 02. 2021  12:19    <JUNCTION>     cygwin64 [C:\cygwin64]
> > 
> > > C:\cygwin64\bin\bash -lc /cygdrive/w/new
> > / -> /
> > 
> > > D:\cygwin64\bin\bash -lc /cygdrive/w/new
> > / -> /cygdrive/c/cygwin64
> 
> This isn't realpath's fault.  You're circumventing the mount point
> handling which is automated in terms of /, depending on the path
> returned from GetModuleFileNameW for the Cygwin DLL.  Since you're
> calling D:\cygwin64\bin\bash the dir returned from GetModuleFileNameW is
> D:\cygwin64\bin, thus root is D:\cygwin64.
> 
> However, junctions are treated as symlinks in Cygwin.  Thus the result
> you see above.

A potential workaround is this:

diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc
index 3a32c352d4e9..01b49468e804 100644
--- a/winsup/cygwin/cygheap.cc
+++ b/winsup/cygwin/cygheap.cc
@@ -144,6 +144,19 @@ init_cygheap::init_installation_root ()
     api_fatal ("Can't initialize Cygwin installation root dir.\n"
 	       "GetModuleFileNameW(%p, %p, %u), %E",
 	       cygwin_hmodule, installation_root_buf, PATH_MAX);
+
+  /* We don't care if fetching the final pathname fails, it's non-fatal and
+     the path returned by GetModuleFileNameW is still valid. */
+  HANDLE h;
+  h = CreateFileW (installation_root_buf, GENERIC_READ, FILE_SHARE_VALID_FLAGS,
+		   &sec_none, OPEN_EXISTING, 0, 0);
+  if (h != INVALID_HANDLE_VALUE)
+    {
+      GetFinalPathNameByHandleW (h, installation_root_buf, PATH_MAX,
+				 FILE_NAME_NORMALIZED);
+      CloseHandle (h);
+    }
+
   PWCHAR p = installation_root_buf;
   if (wcsncasecmp (p, L"\\\\", 2))	/* Normal drive letter path */
     {

I just wonder if such a bordercase is worth the extra performance hit
for each Cygwin invocation.


Corinna

Re: Root directory being a junction fools realpath()

[David Macek]

Thank you for diagnosing.

> > This isn't realpath's fault.  You're circumventing the mount point
> > handling

It was not my intention. :)  It's just due to my sharing scripts and
configurations between various machines (with varying disk layouts and
sizes), I sometimes need to use links to work around things.

[...]

> I just wonder if such a bordercase is worth the extra performance hit
> for each Cygwin invocation.

Of course I'd like to see it resolved, but I don't remember any
non-cosmetic issue caused by it, *so far*.  Is there an estimate of
how big of a slow-down the fix causes?

-- 
David Macek

Re: Root directory being a junction fools realpath()

[Corinna Vinschen]

On Feb 18 12:46, David Macek via Cygwin-developers wrote:
> Thank you for diagnosing.
> 
> > > This isn't realpath's fault.  You're circumventing the mount point
> > > handling
> 
> It was not my intention. :)  It's just due to my sharing scripts and
> configurations between various machines (with varying disk layouts and
> sizes), I sometimes need to use links to work around things.
> 
> [...]
> 
> > I just wonder if such a bordercase is worth the extra performance hit
> > for each Cygwin invocation.
> 
> Of course I'd like to see it resolved, but I don't remember any
> non-cosmetic issue caused by it, *so far*.  Is there an estimate of
> how big of a slow-down the fix causes?

No, somebody would have to test this.  Probably it's not that much...


Corinna

Re: Root directory being a junction fools realpath()

[Corinna Vinschen]

On Feb 18 14:09, Corinna Vinschen via Cygwin-developers wrote:
> On Feb 18 12:46, David Macek via Cygwin-developers wrote:
> > Thank you for diagnosing.
> > 
> > > > This isn't realpath's fault.  You're circumventing the mount point
> > > > handling
> > 
> > It was not my intention. :)  It's just due to my sharing scripts and
> > configurations between various machines (with varying disk layouts and
> > sizes), I sometimes need to use links to work around things.
> > 
> > [...]
> > 
> > > I just wonder if such a bordercase is worth the extra performance hit
> > > for each Cygwin invocation.
> > 
> > Of course I'd like to see it resolved, but I don't remember any
> > non-cosmetic issue caused by it, *so far*.  Is there an estimate of
> > how big of a slow-down the fix causes?
> 
> No, somebody would have to test this.  Probably it's not that much...

I pushed the patch.  New snapshots are in the works, should only take
a couple of minutes.


Corinna

Re: Root directory being a junction fools realpath()

[David Macek]

> I pushed the patch.  New snapshots are in the works, should only take
> a couple of minutes.
>
> Corinna

I tested the patch in a local build and it seems to fix the issue. Thank you.

-- 
David Macek