* dir.cc (mkdir): Check last path component for "..".

	(rmdir): Don't check last path component for "..".
	* fhandler_disk_file.cc (fhandler_disk_file::rmdir): Drop kludge
	which tries to allow deleting the current working directory.
	* path.cc (has_dot_last_component): Add parameter to indicate testing
	for "..".  Take trailing slash into account.
	(symlink_info::posixify): Rely on cygheap->cwd.win32 having a
	useful value.
	(cwdstuff::init): Initialize cygheap->cwd with current working
	directory.  Change to windows_system_directory afterwards.
	(cwdstuff::set): Never call SetCurrentDirectory here.  Just check
	if changing into target directory would be allowed.  Add comment to
	explain why.
	* path.h (has_dot_last_component): Declare with second parameter.
	* pinfo.cc (pinfo::zap_cwd): Remove.
	(pinfo::exit): Drop call to zap_cwd.
	* pinfo.h (class pinfo): Remove declaration of zap_cwd.
	* spawn.cc (spawn_guts): Set current working directory for non-Cygwin
	child applications.  Drop call to zap_cwd.

8 files changed, 118 insertions, 97 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index b8aeca83c..8e7781469 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,25 @@
+2006-11-29  Corinna Vinschen  <corinna@vinschen.de>
+
+	* dir.cc (mkdir): Check last path component for "..".
+	(rmdir): Don't check last path component for "..".
+	* fhandler_disk_file.cc (fhandler_disk_file::rmdir): Drop kludge
+	which tries to allow deleting the current working directory.
+	* path.cc (has_dot_last_component): Add parameter to indicate testing
+	for "..".  Take trailing slash into account.
+	(symlink_info::posixify): Rely on cygheap->cwd.win32 having a
+	useful value.
+	(cwdstuff::init): Initialize cygheap->cwd with current working
+	directory.  Change to windows_system_directory afterwards.
+	(cwdstuff::set): Never call SetCurrentDirectory here.  Just check
+	if changing into target directory would be allowed.  Add comment to
+	explain why.
+	* path.h (has_dot_last_component): Declare with second parameter.
+	* pinfo.cc (pinfo::zap_cwd): Remove.
+	(pinfo::exit): Drop call to zap_cwd.
+	* pinfo.h (class pinfo): Remove declaration of zap_cwd.
+	* spawn.cc (spawn_guts): Set current working directory for non-Cygwin
+	child applications.  Drop call to zap_cwd.
+
 2006-11-28  Corinna Vinschen  <corinna@vinschen.de>
 
 	* security.cc (create_token): Revert erroneous change to test
diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
index 3c94b8c88..c3b22c66d 100644
--- a/winsup/cygwin/dir.cc
+++ b/winsup/cygwin/dir.cc
@@ -277,7 +277,7 @@ mkdir (const char *dir, mode_t mode)
       debug_printf ("got %d error from build_fh_name", fh->error ());
       set_errno (fh->error ());
     }
-  else if (has_dot_last_component (dir))
+  else if (has_dot_last_component (dir, true))
     set_errno (fh->exists () ? EEXIST : ENOENT);
   else if (!fh->mkdir (mode))
     res = 0;
@@ -307,7 +307,7 @@ rmdir (const char *dir)
       debug_printf ("got %d error from build_fh_name", fh->error ());
       set_errno (fh->error ());
     }
-  else if (has_dot_last_component (dir))
+  else if (has_dot_last_component (dir, false))
     set_errno (fh->exists () ? EINVAL : ENOENT);
   else if (!fh->rmdir ())
     res = 0;
diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index f3b5ec03e..2d6ae3578 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1449,70 +1449,42 @@ fhandler_disk_file::rmdir ()
     SetFileAttributes (get_win32_name (),
 		       (DWORD) pc & ~FILE_ATTRIBUTE_READONLY);
 
-  for (bool is_cwd = false; ; is_cwd = true)
-    {
-      DWORD err, att = 0;
-      int rc = RemoveDirectory (get_win32_name ());
+  DWORD err, att = 0;
+  int rc = RemoveDirectory (get_win32_name ());
 
-      if (isremote () && exists ())
-	att = GetFileAttributes (get_win32_name ());
+  if (isremote () && exists ())
+    att = GetFileAttributes (get_win32_name ());
 
-      /* Sometimes smb indicates failure when it really succeeds, so check for
-	 this case specifically. */
-      if (rc || att == INVALID_FILE_ATTRIBUTES)
-	{
-	  /* RemoveDirectory on a samba drive doesn't return an error if the
-	     directory can't be removed because it's not empty. Checking for
-	     existence afterwards keeps us informed about success. */
-	  if (!isremote () || att == INVALID_FILE_ATTRIBUTES)
-	    {
-	      res = 0;
-	      break;
-	    }
-	  err = ERROR_DIR_NOT_EMPTY;
-	}
-      else
-	err = GetLastError ();
-
-      /* This kludge detects if we are attempting to remove the current working
-	 directory.  If so, we will move elsewhere to potentially allow the
-	 rmdir to succeed.  This means that cygwin's concept of the current
-	 working directory != Windows concept but, hey, whaddaregonnado?
-	 Note that this will not cause something like the following to work:
-	 $ cd foo
-	 $ rmdir .
-	 since the shell will have foo "open" in the above case and so Windows
-	 will not allow the deletion. (Actually it does on 9X.)
-	 FIXME: A potential workaround for this is for cygwin apps to *never*
-	 call SetCurrentDirectory. */
-
-      extern char windows_system_directory[];
-      if (strcasematch (get_win32_name (), cygheap->cwd.win32)
-	  && !strcasematch (windows_system_directory, cygheap->cwd.win32)
-	  && !is_cwd
-	  && SetCurrentDirectory (windows_system_directory))
-	continue;
-
-      /* On 9X ERROR_ACCESS_DENIED is returned if you try to remove a
-	 non-empty directory. */
-      if (err == ERROR_ACCESS_DENIED
-	  && wincap.access_denied_on_delete ())
-	err = ERROR_DIR_NOT_EMPTY;
-      /* ...and, that's *not* funny, when trying to remove a non-existing
-	 directory on a share, which is hosted by a 9x machine, the error
-	 code ERROR_INVALID_FUNCTION is returned.  */
-      else if (err == ERROR_INVALID_FUNCTION)
-	err = ERROR_FILE_NOT_FOUND;
-
-      __seterrno_from_win_error (err);
-
-      /* Directory still exists, restore its characteristics. */
-      if (pc.has_attribute (FILE_ATTRIBUTE_READONLY))
-	SetFileAttributes (get_win32_name (), (DWORD) pc);
-      if (is_cwd)
-	SetCurrentDirectory (get_win32_name ());
-      break;
+  /* Sometimes smb indicates failure when it really succeeds, so check for
+     this case specifically. */
+  if (rc || att == INVALID_FILE_ATTRIBUTES)
+    {
+      /* RemoveDirectory on a samba drive doesn't return an error if the
+	 directory can't be removed because it's not empty. Checking for
+	 existence afterwards keeps us informed about success. */
+      if (!isremote () || att == INVALID_FILE_ATTRIBUTES)
+	return res;
+
+      err = ERROR_DIR_NOT_EMPTY;
     }
+  else
+    err = GetLastError ();
+  /* On 9X ERROR_ACCESS_DENIED is returned if you try to remove a
+     non-empty directory. */
+  if (err == ERROR_ACCESS_DENIED
+      && wincap.access_denied_on_delete ())
+    err = ERROR_DIR_NOT_EMPTY;
+  /* ...and, that's *not* funny, when trying to remove a non-existing
+     directory on a share, which is hosted by a 9x machine, the error
+     code ERROR_INVALID_FUNCTION is returned.  */
+  else if (err == ERROR_INVALID_FUNCTION)
+    err = ERROR_FILE_NOT_FOUND;
+
+  __seterrno_from_win_error (err);
+
+  /* Directory still exists, restore its characteristics. */
+  if (pc.has_attribute (FILE_ATTRIBUTE_READONLY))
+    SetFileAttributes (get_win32_name (), (DWORD) pc);
 
   return res;
 }
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index fbd15e667..3012e2564 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -215,16 +215,28 @@ pathmatch (const char *path1, const char *path2)
    Right now, normalize_posix_path will just normalize
    those components away, which changes the semantics.  */
 bool
-has_dot_last_component (const char *dir)
+has_dot_last_component (const char *dir, bool test_dot_dot)
 {
   /* SUSv3: . and .. are not allowed as last components in various system
      calls.  Don't test for backslash path separator since that's a Win32
      path following Win32 rules. */
   const char *last_comp = strrchr (dir, '/');
-  return last_comp
-	 && last_comp[1] == '.'
-	 && (last_comp[2] == '\0'
-	     || (last_comp[2] == '.' && last_comp[3] == '\0'));
+  if (!last_comp)
+    last_comp = dir;
+  else {
+    /* Check for trailing slash.  If so, hop back to the previous slash. */
+    if (!last_comp[1])
+      while (last_comp > dir)
+	if (*--last_comp == '/')
+	  break;
+    if (*last_comp == '/')
+      ++last_comp;
+  }
+  return last_comp[0] == '.'
+	 && ((last_comp[1] == '\0' || last_comp[1] == '/')
+	     || (test_dot_dot
+		 && last_comp[1] == '.'
+		 && (last_comp[2] == '\0' || last_comp[2] == '/')));
 }
 
 #define isslash(c) ((c) == '/')
@@ -3199,10 +3211,7 @@ symlink_info::posixify (char *srcbuf)
         {
 	  char cvtbuf[CYG_MAX_PATH + 6];
 
-	  if (cygheap->cwd.win32)
-	    strncpy (cvtbuf, cygheap->cwd.win32, 2);
-	  else
-	    GetCurrentDirectory (CYG_MAX_PATH, cvtbuf);
+	  strncpy (cvtbuf, cygheap->cwd.win32, 2);
 	  strcpy (cvtbuf + 2, srcbuf);
 	  mount_table->conv_to_posix_path (cvtbuf, contents, 0);
 	}
@@ -4146,6 +4155,12 @@ void
 cwdstuff::init ()
 {
   cwd_lock.init ("cwd_lock");
+  get_initial ();
+  /* Actually chdir into the syste dir to avoid cwd problems.  See comment
+     in cwdstuff::set below. */
+  extern char windows_system_directory[];
+  SetCurrentDirectory (windows_system_directory);
+  cwd_lock.release ();
 }
 
 /* Get initial cwd.  Should only be called once in a
@@ -4173,17 +4188,42 @@ cwdstuff::set (const char *win32_cwd, const char *posix_cwd, bool doit)
 
   if (win32_cwd)
     {
-       cwd_lock.acquire ();
-       if (doit && !SetCurrentDirectory (win32_cwd))
-	 {
-	    /* When calling SetCurrentDirectory for a non-existant dir on a
-	       Win9x share, it returns ERROR_INVALID_FUNCTION. */
-	    if (GetLastError () == ERROR_INVALID_FUNCTION)
+      cwd_lock.acquire ();
+      if (doit)
+        {
+	  /* Check if we *could* chdir, if we actually would.
+	  
+	     Why don't we actually chdir?  For two reasons:
+	     - A process has always an open handle to the current working
+	       directory which disallows manipulating this directory.
+	       POSIX allows to remove a directory if the permissions are
+	       ok.  The fact that its the cwd of some process doesn't matter.
+	     - SetCurrentDirectory fails for directories with strict
+	       permissions even for processes with the SE_BACKUP_NAME
+	       privilege enabled.  The reason is apparently that
+	       SetCurrentDirectory calls NtOpenFile without the
+	       FILE_OPEN_FOR_BACKUP_INTENT flag set. */
+	  DWORD attr = GetFileAttributes (win32_cwd);
+	  if (attr == INVALID_FILE_ATTRIBUTES)
+	    {
 	      set_errno (ENOENT);
-	    else
+	      goto out;
+	    }
+	  if (!(attr & FILE_ATTRIBUTE_DIRECTORY))
+	    {
+	      set_errno (ENOTDIR);
+	      goto out;
+	    }
+	  HANDLE h = CreateFile (win32_cwd, GENERIC_READ, wincap.shared (),
+				 NULL, OPEN_EXISTING,
+				 FILE_FLAG_BACKUP_SEMANTICS, NULL);
+	  if (h == INVALID_HANDLE_VALUE)
+	    {
 	      __seterrno ();
-	    goto out;
-	 }
+	      goto out;
+	    }
+	  CloseHandle (h);
+        }
     }
   /* If there is no win32 path or it has the form c:xxx, get the value */
   if (!win32_cwd || (isdrive (win32_cwd) && win32_cwd[2] != '\\'))
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index 4097053c7..e011575f2 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -305,7 +305,7 @@ has_exec_chars (const char *buf, int len)
 
 int pathmatch (const char *path1, const char *path2) __attribute__ ((regparm (2)));
 int pathnmatch (const char *path1, const char *path2, int len) __attribute__ ((regparm (2)));
-bool has_dot_last_component (const char *dir) __attribute__ ((regparm (1)));
+bool has_dot_last_component (const char *dir, bool test_dot_dot) __attribute__ ((regparm (2)));
 
 bool fnunmunge (char *, const char *) __attribute__ ((regparm (2)));
 
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index d73ad20aa..404591b26 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -123,16 +123,6 @@ pinfo::maybe_set_exit_code_from_windows ()
 }
 
 void
-pinfo::zap_cwd ()
-{
-  extern char windows_system_directory[];
-  /* Move to an innocuous location to avoid a race with other processes
-     that may want to manipulate the current directory before this
-     process has completely exited.  */
-  SetCurrentDirectory (windows_system_directory);
-}
-
-void
 pinfo::exit (DWORD n)
 {
   minimal_printf ("winpid %d, exit %d", GetCurrentProcessId (), n);
@@ -148,7 +138,6 @@ pinfo::exit (DWORD n)
     }
 
   sigproc_terminate (ES_FINAL);
-  zap_cwd ();
 
   /* FIXME:  There is a potential race between an execed process and its
      parent here.  I hated to add a mutex just for that, though.  */
diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h
index 0367f38e1..7d32f0bd5 100644
--- a/winsup/cygwin/pinfo.h
+++ b/winsup/cygwin/pinfo.h
@@ -193,7 +193,6 @@ public:
 #endif
   HANDLE shared_handle () {return h;}
   void set_acl ();
-  void zap_cwd ();
   friend class _pinfo;
   friend class winpids;
 };
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 2771d87e0..d1750f744 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -501,7 +501,7 @@ loop:
 			  TRUE,		/* inherit handles from parent */
 			  c_flags,
 			  envblock,	/* environment */
-			  0,		/* use current drive/directory */
+			  real_path.iscygexec () ? NULL : cygheap->cwd.win32,
 			  &si,
 			  &pi);
     }
@@ -536,7 +536,7 @@ loop:
 		       TRUE,		/* inherit handles from parent */
 		       c_flags,
 		       envblock,	/* environment */
-		       0,		/* use current drive/directory */
+		       real_path.iscygexec () ? NULL : cygheap->cwd.win32,
 		       &si,
 		       &pi);
     }
@@ -613,7 +613,6 @@ loop:
 	      myself->sync_proc_pipe ();	/* Make sure that we own wr_proc_pipe
 						   just in case we've been previously
 						   execed. */
-	      myself.zap_cwd ();
 	    }
 	  orig_wr_proc_pipe = myself->dup_proc_pipe (pi.hProcess);
 	}