* 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.

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 0902b27..0cb0217 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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
 };