chown

[Pierre A. Humblet]

At 11:37 PM 4/16/2004 +0200, Corinna Vinschen wrote:
>On Apr 16 10:28, Pierre A. Humblet wrote:
>> Corinna Vinschen wrote:
>> By getting RESTORE_NAME earlier we can avoid the fallback to 
>> query_read_control.
>
>That's done now.
>
>> No advantage if you call CreateFile. But at that time I was still
>> pursuing the idea of DuplicateHandle and I was wondering if having
>> FILE_FLAG_BACKUP_SEMANTICS in the initial open was necessary. 
>> MS is not clear about combinations work or don't work.
>
>Yes, MS is not clear.  Simply scratch the rest of the sentence.
>
>> I have now removed the test (owner != cygheap->user.sid ()) in write_sd
>> and don't observe problems. NtSetSecurityObject appears to be atomic.
>
>I've removed the test entirely.  The restore privilege is now requested
>where it's really needed, always before opening the file.
>
>I've changed fhandler_base::open to use NtCreateFile now.  After some
>head scratching and searching with google, I read that the Win32 CreateFile
>call adds some access bits at its own will, namely the FILE_READ_ATTRIBUTES
>and SYNCHRONIZE bits.  The latter is a problem when a user has no
>FILE_READ_DATA permission on a file since that apparently seem to disallow
>requesting SYNCHRONIZE. 
>I hope I got everything right.  I'm not sure about serial I/O, since it's
>not entirely clear to me if FILE_FLAG_OVERLAPPED is simply translatable
>into not setting the FILE_SYNCHRONOUS_IO_NONALERT flag.

Wow, why the change to NtCreateFile? It's unlikely to be chown/chmod.
Everything looks OK, I just have a few comments.  
- Attached is a diff that streamlines the processing on 9X and when ntsec 
is off. 
- I noticed that the fchmod method will return success even when ntea is
present and fails, but that's inherited from the old chmod.
-I don't see why it's necessary to add FILE_ATTRIBUTE_SYSTEM in the fchmod
method. It should be present already.
- The new open_9x contains ntsec related stuff that could be removed.

Pierre
-------------- next part --------------
Index: fhandler_disk_file.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_disk_file.cc,v
retrieving revision 1.89
diff -u -p -r1.89 fhandler_disk_file.cc
--- fhandler_disk_file.cc	16 Apr 2004 21:22:13 -0000	1.89
+++ fhandler_disk_file.cc	17 Apr 2004 00:44:57 -0000
@@ -387,16 +387,19 @@ fhandler_disk_file::fchmod (mode_t mode)
 	  if (!(oret = open_fs (O_BINARY, 0)))
 	    return -1;
 	}
-    }

-  if (!allow_ntsec && allow_ntea) /* Not necessary when manipulating SD. */
-    SetFileAttributes (pc, (DWORD) pc & ~FILE_ATTRIBUTE_READONLY);
-  if (pc.isdir ())
-    mode |= S_IFDIR;
-  if (!set_file_attribute (pc.has_acls (), get_io_handle (), pc,
-			   ILLEGAL_UID, ILLEGAL_GID, mode)
-      && allow_ntsec)
-    res = 0;
+      if (!allow_ntsec && allow_ntea) /* Not necessary when manipulating SD. */
+	SetFileAttributes (pc, (DWORD) pc & ~FILE_ATTRIBUTE_READONLY);
+      if (pc.isdir ())
+	mode |= S_IFDIR;
+      if (!set_file_attribute (pc.has_acls (), get_io_handle (), pc,
+			       ILLEGAL_UID, ILLEGAL_GID, mode)
+	  && allow_ntsec)
+	res = 0;
+
+      if (oret)
+	close_fs ();
+    }

   /* if the mode we want has any write bits set, we can't be read only. */
   if (mode & (S_IWUSR | S_IWGRP | S_IWOTH))
@@ -413,9 +416,6 @@ fhandler_disk_file::fchmod (mode_t mode)
     /* Correct NTFS security attributes have higher priority */
     res = 0;

-  if (oret)
-    close_fs ();
-
   return res;
 }

@@ -424,6 +424,13 @@ fhandler_disk_file::fchown (__uid32_t ui
 {
   int oret = 0;

+  if (!pc.has_acls () || !allow_ntsec)
+    {
+      /* fake - if not supported, pretend we're like win95
+         where it just works */
+      return 0;
+    }
+
   enable_restore_privilege ();
   if (!get_io_handle ())
     {
@@ -439,12 +446,6 @@ fhandler_disk_file::fchown (__uid32_t ui
   if (!res)
     res = set_file_attribute (pc.has_acls (), get_io_handle (), pc,
 			      uid, gid, attrib);
-  if (res && (!pc.has_acls () || !allow_ntsec))
-    {
-      /* fake - if not supported, pretend we're like win95
-         where it just works */
-      res = 0;
-    }

   if (oret)
     close_fs ();