* fhandler.cc (fhandler_base::open): When creating a file on a

	filesystem supporting ACLs, create the file with WRITE_DAC access.
	Explain why.
	* fhandler_disk_file.cc (fhandler_disk_file::mkdir): Ditto for
	directories.
	* fhandler_socket.cc (fhandler_socket::bind): Ditto for sockets.
	* path.cc (symlink_worker): Ditto for symlinks.
	* security.cc (get_file_sd): Always call GetSecurityInfo for directories
	on XP and Server 2003.  Improve comment to explain why.
	(set_file_attribute): Explicitely cast mode_t value to bool in call to
	get_file_sd.
	* wincap.h (wincaps::use_get_sec_info_on_dirs): New element.
	* wincap.cc: Implement above element throughout.

8 files changed, 85 insertions, 21 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 0902b27..0cb0217 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,19 @@
+2011-03-07  Corinna Vinschen  <corinna@vinschen.de>
+
+	* fhandler.cc (fhandler_base::open): When creating a file on a
+	filesystem supporting ACLs, create the file with WRITE_DAC access.
+	Explain why.
+	* fhandler_disk_file.cc (fhandler_disk_file::mkdir): Ditto for
+	directories.
+	* fhandler_socket.cc (fhandler_socket::bind): Ditto for sockets.
+	* path.cc (symlink_worker): Ditto for symlinks.
+	* security.cc (get_file_sd): Always call GetSecurityInfo for directories
+	on XP and Server 2003.  Improve comment to explain why.
+	(set_file_attribute): Explicitely cast mode_t value to bool in call to
+	get_file_sd.
+	* wincap.h (wincaps::use_get_sec_info_on_dirs): New element.
+	* wincap.cc: Implement above element throughout.
+
 2011-03-04  Corinna Vinschen  <corinna@vinschen.de>
 
 	* fhandler_procsys.cc (fhandler_procsys::exists): Rewrite.
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index cea9c1b..963635c 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -577,6 +577,13 @@ fhandler_base::open (int flags, mode_t mode)
 	    /* If mode has no write bits set, and ACLs are not used, we set
 	       the DOS R/O attribute. */
 	    file_attributes |= FILE_ATTRIBUTE_READONLY;
+	  else if (!exists () && has_acls ())
+	    /* If we are about to create the file and the filesystem supports
+	       ACLs, we will overwrite the DACL after the call to NtCreateFile.
+	       This requires a handle with additional WRITE_DAC access,
+	       otherwise set_file_sd has to open the file again. */
+	    access |= WRITE_DAC;
+
 	  /* The file attributes are needed for later use in, e.g. fchmod. */
 	  pc.file_attributes (file_attributes);
 	}
diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index 115b746..cb85fee 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1460,6 +1460,7 @@ fhandler_disk_file::mkdir (mode_t mode)
   IO_STATUS_BLOCK io;
   PFILE_FULL_EA_INFORMATION p = NULL;
   ULONG plen = 0;
+  ULONG access = FILE_LIST_DIRECTORY | SYNCHRONIZE;
 
   if (pc.fs_is_nfs ())
     {
@@ -1479,8 +1480,13 @@ fhandler_disk_file::mkdir (mode_t mode)
       nfs_attr->type = NF3DIR;
       nfs_attr->mode = (mode & 07777) & ~cygheap->umask;
     }
-  status = NtCreateFile (&dir, FILE_LIST_DIRECTORY | SYNCHRONIZE,
-			 pc.get_object_attr (attr, sa), &io, NULL,
+  else if (has_acls ())
+    /* If the filesystem supports ACLs, we will overwrite the DACL after the
+       call to NtCreateFile.  This requires a handle with READ_CONTROL and
+       WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to
+       open the file again. */
+    access |= READ_CONTROL | WRITE_DAC;
+  status = NtCreateFile (&dir, access, pc.get_object_attr (attr, sa), &io, NULL,
 			 FILE_ATTRIBUTE_DIRECTORY, FILE_SHARE_VALID_FLAGS,
 			 FILE_CREATE,
 			 FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT
diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 1b4bf29..22ae05f 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -996,10 +996,17 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
       HANDLE fh;
       OBJECT_ATTRIBUTES attr;
       IO_STATUS_BLOCK io;
+      ULONG access = DELETE | FILE_GENERIC_WRITE;
 
-      status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE,
-			     pc.get_object_attr (attr, sa), &io, NULL, fattr,
-			     0, FILE_CREATE,
+      /* If the filesystem supports ACLs, we will overwrite the DACL after the
+	 call to NtCreateFile.  This requires a handle with READ_CONTROL and
+	 WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to
+	 open the file again. */
+      if (pc.has_acls ())
+	access |= READ_CONTROL | WRITE_DAC;
+
+      status = NtCreateFile (&fh, access, pc.get_object_attr (attr, sa), &io,
+			     NULL, fattr, 0, FILE_CREATE,
 			     FILE_NON_DIRECTORY_FILE
 			     | FILE_SYNCHRONOUS_IO_NONALERT
 			     | FILE_OPEN_FOR_BACKUP_INTENT,
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index f6704fb..4a7c116 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -1415,6 +1415,7 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
   IO_STATUS_BLOCK io;
   NTSTATUS status;
   HANDLE fh;
+  ULONG access = DELETE | FILE_GENERIC_WRITE;
   tmp_pathbuf tp;
   unsigned check_opt;
   bool mk_winsym = use_winsym;
@@ -1671,8 +1672,14 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
 	  goto done;
 	}
     }
-  status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE,
-			 win32_newpath.get_object_attr (attr, sa),
+  else if (!isdevice && win32_newpath.has_acls ())
+    /* If the filesystem supports ACLs, we will overwrite the DACL after the
+       call to NtCreateFile.  This requires a handle with READ_CONTROL and
+       WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to
+       open the file again. */
+    access |= READ_CONTROL | WRITE_DAC;
+
+  status = NtCreateFile (&fh, access, win32_newpath.get_object_attr (attr, sa),
 			 &io, NULL, FILE_ATTRIBUTE_NORMAL,
 			 FILE_SHARE_VALID_FLAGS,
 			 isdevice ? FILE_OVERWRITE_IF : FILE_CREATE,
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index 73b740c..6d8d6db 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -43,21 +43,31 @@ get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd,
     {
       if (fh)
 	{
-	  if (justcreated)
+	  /* Amazing but true.  If you want to know if an ACE is inherited
+	     from the parent object, you can't use the NtQuerySecurityObject
+	     function.  In the DACL returned by this functions, the
+	     INHERITED_ACE flag is never set.  Only by calling GetSecurityInfo
+	     you get this information.
+	     
+	     However, this functionality is slow, and the extra information is
+	     only required when the file has been created and the permissions
+	     are about to be set to POSIX permissions.  Therefore we only use
+	     it in case the file just got created.  In all other cases we
+	     rather call NtQuerySecurityObject directly...
+	     
+	     ...except that there's a problem on 5.1 and 5.2 kernels.  The
+	     GetSecurityInfo call on a file sometimes returns with
+	     ERROR_INVALID_ADDRESS if a former request for the SD of the
+	     parent directory (or one of the parent directories?) used the
+	     NtQuerySecurityObject call, rather than GetSecurityInfo as well.
+	     As soon as all directory SDs are fetched using GetSecurityInfo,
+	     the problem disappears. */
+	  if (justcreated
+	      || (pc.isdir () && wincap.use_get_sec_info_on_dirs ()))
 	    {
-	      /* Amazing but true.  If you want to know if an ACE is inherited
-		 from the parent object, you can't use the NtQuerySecurityObject
-		 function.  In the DACL returned by this functions, the
-		 INHERITED_ACE flag is never set.  Only by calling
-		 GetSecurityInfo you get this information.
-		 
-		 This functionality is slow, and the extra information is only
-		 required when the file has been created and the permissions
-		 are about to be set to POSIX permissions.  Therefore we only
-		 use it in case the file just got created.  In all other cases
-		 we rather call NtQuerySecurityObject directly. */
 	      PSECURITY_DESCRIPTOR psd;
-	      error = GetSecurityInfo (fh, SE_FILE_OBJECT, ALL_SECURITY_INFORMATION,
+	      error = GetSecurityInfo (fh, SE_FILE_OBJECT,
+				       ALL_SECURITY_INFORMATION,
 				       NULL, NULL, NULL, NULL, &psd);
 	      if (error == ERROR_SUCCESS)
 		{
@@ -876,7 +886,7 @@ set_file_attribute (HANDLE handle, path_conv &pc,
     {
       security_descriptor sd;
 
-      if (!get_file_sd (handle, pc, sd, attribute & S_JUSTCREATED)
+      if (!get_file_sd (handle, pc, sd, (bool)(attribute & S_JUSTCREATED))
 	  && alloc_sd (pc, uid, gid, attribute, sd))
 	ret = set_file_sd (handle, pc, sd,
 			   uid != ILLEGAL_UID || gid != ILLEGAL_GID);
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index 89d0323..c153721 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -62,6 +62,7 @@ wincaps wincap_nt4sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -103,6 +104,7 @@ wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -144,6 +146,7 @@ wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -185,6 +188,7 @@ wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
+  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -226,6 +230,7 @@ wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
+  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -267,6 +272,7 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
+  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -308,6 +314,7 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -349,6 +356,7 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:true,
   has_restricted_raw_disk_access:true,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -390,6 +398,7 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:true,
   has_restricted_raw_disk_access:true,
   use_dont_resolve_hack:false,
+  use_get_sec_info_on_dirs:false,
 };
 
 wincapc wincap __attribute__((section (".cygwin_dll_common"), shared));
diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h
index f1eb7a6..60d0512 100644
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -52,6 +52,7 @@ struct wincaps
   unsigned has_fast_cwd					: 1;
   unsigned has_restricted_raw_disk_access		: 1;
   unsigned use_dont_resolve_hack			: 1;
+  unsigned use_get_sec_info_on_dirs			: 1;
 };
 
 class wincapc
@@ -109,6 +110,7 @@ public:
   bool	IMPLEMENT (has_fast_cwd)
   bool	IMPLEMENT (has_restricted_raw_disk_access)
   bool	IMPLEMENT (use_dont_resolve_hack)
+  bool	IMPLEMENT (use_get_sec_info_on_dirs)
 
 #undef IMPLEMENT
 };