* autoload.cc (PrivilegeCheck): Define.

	* fhandler.cc (fhandler_base::open): Always try opening with backup
	resp. restore intent.
	* fhandler_disk_file.cc (fhandler_disk_file::opendir): Always try
	opening with backup intent.
	(fhandler_disk_file::readdir): Ditto when trying to retrieve file id
	explicitely.
	* security.cc (check_file_access): Replace pbuf with correctly
	PPRIVILEGE_SET typed pset.  Check explicitely for backup and/or restore
	privileges when AccessCheck fails, to circumvent AccessCheck
	shortcoming.  Add comment to explain.

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 0d4763e..5cd5203 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,17 @@
+2006-10-13  Corinn6 Vinschen  <corinna@vinschen.de>
+
+       * autoload.cc (PrivilegeCheck): Define.
+       * fhandler.cc (fhandler_base::open): Always try opening with backup
+       resp. restore intent.
+       * fhandler_disk_file.cc (fhandler_disk_file::opendir): Always try
+       opening with backup intent.
+       (fhandler_disk_file::readdir): Ditto when trying to retrieve file id
+       explicitely.
+       * security.cc (check_file_access): Replace pbuf with correctly 
+       PPRIVILEGE_SET typed pset.  Check explicitely for backup and/or restore
+       privileges when AccessCheck fails, to circumvent AccessCheck
+       shortcoming.  Add comment to explain.
+
 2006-10-13  Christopher Faylor  <cgf@timesys.com>
 
        * winsup.h: Turn off DEBUGGING.

diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc
index 63a83d8..738d832 100644 (file)
--- a/winsup/cygwin/autoload.cc
+++ b/winsup/cygwin/autoload.cc
@@ -347,6 +347,7 @@ LoadDLLfunc (LsaQueryInformationPolicy, 12, advapi32)
 LoadDLLfunc (MakeSelfRelativeSD, 12, advapi32)
 LoadDLLfunc (OpenProcessToken, 12, advapi32)
 LoadDLLfunc (OpenThreadToken, 16, advapi32)
+LoadDLLfunc (PrivilegeCheck, 12, advapi32)
 // LoadDLLfunc (RegCloseKey, 4, advapi32)
 LoadDLLfunc (RegCreateKeyExA, 36, advapi32)
 LoadDLLfunc (RegDeleteKeyA, 8, advapi32)

diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 00e9875..672b434 100644 (file)
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -620,13 +620,22 @@ fhandler_base::open (int flags, mode_t mode)
        create_options = FILE_OPEN_FOR_BACKUP_INTENT | FILE_OPEN_FOR_RECOVERY;
        break;
       default:
-       create_options = 0;
        if ((flags & O_ACCMODE) == O_RDONLY)
-         access = GENERIC_READ;
+         {
+           access = GENERIC_READ;
+           create_options = FILE_OPEN_FOR_BACKUP_INTENT;
+         }
        else if ((flags & O_ACCMODE) == O_WRONLY)
-         access = GENERIC_WRITE | FILE_READ_ATTRIBUTES;
+         {
+           access = GENERIC_WRITE | FILE_READ_ATTRIBUTES;
+           create_options = FILE_OPEN_FOR_RECOVERY;
+         }
        else
-         access = GENERIC_READ | GENERIC_WRITE;
+         {
+           access = GENERIC_READ | GENERIC_WRITE;
+           create_options = FILE_OPEN_FOR_BACKUP_INTENT
+                            | FILE_OPEN_FOR_RECOVERY;
+         }
        if (flags & O_SYNC)
          create_options |= FILE_WRITE_THROUGH;
        if (flags & O_DIRECT)

diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index ae2c90d..66f488b 100644 (file)
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -1606,6 +1606,7 @@ fhandler_disk_file::opendir ()
                                   SYNCHRONIZE | FILE_LIST_DIRECTORY,
                                   &attr, &io, wincap.shared (),
                                   FILE_SYNCHRONOUS_IO_NONALERT
+                                  | FILE_OPEN_FOR_BACKUP_INTENT
                                   | FILE_DIRECTORY_FILE);
              if (!NT_SUCCESS (status))
                {
@@ -1869,7 +1870,7 @@ go_ahead:
              InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE,
                                          dir->__handle , NULL);
              if (!NtOpenFile (&hdl, READ_CONTROL, &attr, &io,
-                              wincap.shared (), 0))
+                              wincap.shared (), FILE_OPEN_FOR_BACKUP_INTENT))
                {
                  de->d_ino = readdir_get_ino_by_handle (hdl);
                  CloseHandle (hdl);

diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index a0bcba3..5484b70 100644 (file)
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -1953,8 +1953,9 @@ check_file_access (const char *fn, int flags)
 
   HANDLE hToken;
   BOOL status;
-  char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof (LUID_AND_ATTRIBUTES)];
-  DWORD desired = 0, granted, plength = sizeof pbuf;
+  DWORD desired = 0, granted;
+  DWORD plength = sizeof (PRIVILEGE_SET) + 3 * sizeof (LUID_AND_ATTRIBUTES);
+  PPRIVILEGE_SET pset = (PPRIVILEGE_SET) alloca (plength);
   static GENERIC_MAPPING NO_COPY mapping = { FILE_GENERIC_READ,
                                             FILE_GENERIC_WRITE,
                                             FILE_GENERIC_EXECUTE,
@@ -1974,10 +1975,46 @@ check_file_access (const char *fn, int flags)
   if (flags & X_OK)
     desired |= FILE_EXECUTE;
   if (!AccessCheck (sd, hToken, desired, &mapping,
-                   (PPRIVILEGE_SET) pbuf, &plength, &granted, &status))
+                   pset, &plength, &granted, &status))
     __seterrno ();
   else if (!status)
-    set_errno (EACCES);
+    {
+      /* CV, 2006-10-16: Now, that's really weird.  Imagine a user who has no
+        standard access to a file, but who has backup and restore privileges
+        and these privileges are enabled in the access token.  One would
+        expect that the AccessCheck function takes this into consideration
+        when returning the access status.  Otherwise, why bother with the
+        pset parameter, right?
+        But not so.  AccessCheck actually returns a status of "false" here,
+        even though opening a file with backup resp.  restore intent
+        naturally succeeds for this user.  This definitely spoils the results
+        of access(2) for administrative users or the SYSTEM account.  So, in
+        case the access check fails, another check against the user's
+        backup/restore privileges has to be made.  Sigh. */
+      int granted_flags = 0;
+      if (flags & R_OK)
+        {
+         pset->PrivilegeCount = 1;
+         pset->Control = 0;
+         pset->Privilege[0].Luid = *privilege_luid (SE_BACKUP_PRIV);
+         pset->Privilege[0].Attributes = 0;
+         if (PrivilegeCheck (hToken, pset, &status) && status)
+           granted_flags |= R_OK;
+       }
+      if (flags & W_OK)
+        {
+         pset->PrivilegeCount = 1;
+         pset->Control = 0;
+         pset->Privilege[0].Luid = *privilege_luid (SE_RESTORE_PRIV);
+         pset->Privilege[0].Attributes = 0;
+         if (PrivilegeCheck (hToken, pset, &status) && status)
+           granted_flags |= W_OK;
+       }
+      if (granted_flags == flags)
+        ret = 0;
+      else
+       set_errno (EACCES);
+    }
   else
     ret = 0;
  done: