Cygwin: symlink/mknod: fix ACL handling

mknod32 actually creates a path_conv, just to call mknod_worker
with a win32 path.  This doesn't only require to create path_conv
twice, it also breaks permissions on filesystems supporting ACLs.

Fix this by passing the path_conv created in the caller down to
symlink_worker.  Also, while at it, simplify the handling of trailing
slashes and move it out of symlink_worker.  Especially use the
new PC_SYM_NOFOLLOW_DIR flag to avoid fiddeling with creating
a new path copy without the trailing slash.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

winsup/cygwin/path.cc

@@ -1674,7 +1674,34 @@ conv_path_list (const char *src, char *dst, size_t size,
 extern "C" int
 symlink (const char *oldpath, const char *newpath)
 {
-  return symlink_worker (oldpath, newpath, false);
+  path_conv win32_newpath;
+
+  __try
+    {
+      if (!*oldpath || !*newpath)
+	{
+	  set_errno (ENOENT);
+	  __leave;
+	}
+
+      /* Trailing dirsep is a no-no, only errno differs. */
+      bool has_trailing_dirsep = isdirsep (newpath[strlen (newpath) - 1]);
+      win32_newpath.check (newpath,
+			   PC_SYM_NOFOLLOW | PC_SYM_NOFOLLOW_DIR | PC_POSIX,
+			   stat_suffixes);
+
+      if (win32_newpath.error || has_trailing_dirsep)
+	{
+	  set_errno (win32_newpath.error ?:
+		     win32_newpath.exists () ? EEXIST : ENOENT);
+	  __leave;
+	}
+
+      return symlink_worker (oldpath, win32_newpath, false);
+    }
+  __except (EFAULT) {}
+  __endtry
+  return -1;
 }
 
 static int
@@ -1846,15 +1873,12 @@ symlink_native (const char *oldpath, path_conv &win32_newpath)
 }
 
 int
-symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
+symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice)
 {
   int res = -1;
   size_t len;
-  path_conv win32_newpath;
   char *buf, *cp;
   tmp_pathbuf tp;
-  unsigned check_opt;
-  bool has_trailing_dirsep = false;
   winsym_t wsym_type;
 
   /* POSIX says that empty 'newpath' is invalid input while empty
@@ -1862,31 +1886,12 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
      symlink contents point to existing filesystem object */
   __try
     {
-      if (!*oldpath || !*newpath)
-	{
-	  set_errno (ENOENT);
-	  __leave;
-	}
-
       if (strlen (oldpath) > SYMLINK_MAX)
 	{
 	  set_errno (ENAMETOOLONG);
 	  __leave;
 	}
 
-      /* Trailing dirsep is a no-no. */
-      len = strlen (newpath);
-      has_trailing_dirsep = isdirsep (newpath[len - 1]);
-      if (has_trailing_dirsep)
-	{
-	  newpath = strdup (newpath);
-	  ((char *) newpath)[len - 1] = '\0';
-	}
-
-      check_opt = PC_SYM_NOFOLLOW | PC_POSIX | (isdevice ? PC_NOWARN : 0);
-      /* We need the normalized full path below. */
-      win32_newpath.check (newpath, check_opt, stat_suffixes);
-
       /* Default symlink type is determined by global allow_winsymlinks
 	 variable.  Device files are always shortcuts. */
       wsym_type = isdevice ? WSYM_lnk : allow_winsymlinks;
@@ -1910,8 +1915,9 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
 	  && (isdevice || !win32_newpath.fs_is_nfs ()))
 	{
 	  char *newplnk = tp.c_get ();
-	  stpcpy (stpcpy (newplnk, newpath), ".lnk");
+	  stpcpy (stpcpy (newplnk, win32_newpath.get_posix ()), ".lnk");
-	  win32_newpath.check (newplnk, check_opt);
+	  win32_newpath.check (newplnk, PC_SYM_NOFOLLOW | PC_POSIX
+					| (isdevice ? PC_NOWARN : 0));
 	}
 
       if (win32_newpath.error)
@@ -1929,11 +1935,6 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
 	  set_errno (EEXIST);
 	  __leave;
 	}
-      if (has_trailing_dirsep && !win32_newpath.exists ())
-	{
-	  set_errno (ENOENT);
-	  __leave;
-	}
 
       /* Handle NFS and native symlinks in their own functions. */
       switch (wsym_type)
@@ -2189,9 +2190,7 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice)
   __except (EFAULT) {}
   __endtry
   syscall_printf ("%d = symlink_worker(%s, %s, %d)",
-		  res, oldpath, newpath, isdevice);
+		  res, oldpath, win32_newpath.get_posix (), isdevice);
-  if (has_trailing_dirsep)
-    free ((void *) newpath);
   return res;
 }
 
@@ -3389,7 +3388,7 @@ chdir (const char *in_dir)
       syscall_printf ("dir '%s'", in_dir);
 
       /* Convert path.  PC_NONULLEMPTY ensures that we don't check for
-      	 NULL/empty/invalid again. */
+	 NULL/empty/invalid again. */
       path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
       if (path.error)
 	{

winsup/cygwin/path.h

@@ -447,4 +447,4 @@ int normalize_win32_path (const char *, char *, char *&);
 int normalize_posix_path (const char *, char *, char *&);
 PUNICODE_STRING __reg3 get_nt_native_path (const char *, UNICODE_STRING&, bool);
 
-int __reg3 symlink_worker (const char *, const char *, bool);
+int __reg3 symlink_worker (const char *, path_conv &, bool);

winsup/cygwin/syscalls.cc

@@ -73,8 +73,7 @@ details. */
 #undef _lseek64
 #undef _fstat64
 
-static int __stdcall mknod_worker (const char *, mode_t, mode_t, _major_t,
+static int mknod_worker (path_conv &, mode_t, _major_t, _minor_t);
-				   _minor_t);
 
 /* Close all files and process any queued deletions.
    Lots of unix style applications will open a tmp file, unlink it,
@@ -1773,15 +1772,16 @@ umask (mode_t mask)
   return oldmask;
 }
 
+#define FILTERED_MODE(m)	((m) & (S_ISUID | S_ISGID | S_ISVTX \
+					| S_IRWXU | S_IRWXG | S_IRWXO))
+
 int
 chmod_device (path_conv& pc, mode_t mode)
 {
-  return mknod_worker (pc.get_win32 (), pc.dev.mode () & S_IFMT, mode, pc.dev.get_major (), pc.dev.get_minor ());
+  return mknod_worker (pc, (pc.dev.mode () & S_IFMT) | FILTERED_MODE (mode),
+		       pc.dev.get_major (), pc.dev.get_minor ());
 }
 
-#define FILTERED_MODE(m)	((m) & (S_ISUID | S_ISGID | S_ISVTX \
-					| S_IRWXU | S_IRWXG | S_IRWXO))
-
 /* chmod: POSIX 5.6.4.1 */
 extern "C" int
 chmod (const char *path, mode_t mode)
@@ -3364,14 +3364,12 @@ ptsname_r (int fd, char *buf, size_t buflen)
   return cfd->ptsname_r (buf, buflen);
 }
 
-static int __stdcall
+static int
-mknod_worker (const char *path, mode_t type, mode_t mode, _major_t major,
+mknod_worker (path_conv &pc, mode_t mode, _major_t major, _minor_t minor)
-	      _minor_t minor)
 {
   char buf[sizeof (":\\00000000:00000000:00000000") + PATH_MAX];
-  sprintf (buf, ":\\%x:%x:%x", major, minor,
+  sprintf (buf, ":\\%x:%x:%x", major, minor, mode);
-	   type | (mode & (S_IRWXU | S_IRWXG | S_IRWXO)));
+  return symlink_worker (buf, pc, true);
-  return symlink_worker (buf, path, true);
 }
 
 extern "C" int
@@ -3388,12 +3386,17 @@ mknod32 (const char *path, mode_t mode, dev_t dev)
       if (strlen (path) >= PATH_MAX)
 	__leave;
 
-      path_conv w32path (path, PC_SYM_NOFOLLOW);
+      /* Trailing dirsep is a no-no, only errno differs. */
-      if (w32path.exists ())
+      bool has_trailing_dirsep = isdirsep (path[strlen (path) - 1]);
-	{
+
-	  set_errno (EEXIST);
+      path_conv w32path (path, PC_SYM_NOFOLLOW | PC_SYM_NOFOLLOW_DIR
-	  __leave;
+			       | PC_POSIX, stat_suffixes);
-	}
+
+      if (w32path.exists () || has_trailing_dirsep)
+        {
+          set_errno (w32path.exists () ? EEXIST : ENOENT);
+          __leave;
+        }
 
       mode_t type = mode & S_IFMT;
       _major_t major = _major (dev);
@@ -3424,7 +3427,7 @@ mknod32 (const char *path, mode_t mode, dev_t dev)
 	  __leave;
 	}
 
-      return mknod_worker (w32path.get_win32 (), type, mode, major, minor);
+      return mknod_worker (w32path, mode, major, minor);
     }
   __except (EFAULT)
   __endtry