Partially revert change from 2006-10-22. GetSecurityInfo messes up

	user information on NT4.
	* sec_helper.cc (security_descriptor::malloc): Drop LocalAlloc
	considerations.
	(security_descriptor::realloc): Ditto.
	(security_descriptor::free): Ditto.
	* security.cc (get_reg_security): Reinstantiate.
	(get_nt_object_security): Revert to using NtQuerySecurityObject.
	* security.h (class security_descriptor): Drop type member.
	Accommodate throughout.
	(security_descriptor::size): Constify.
	(security_descriptor::copy): Ditto.

4 files changed, 88 insertions, 46 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index d43f624da..b04ba014f 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,18 @@
+2006-12-20  Corinna Vinschen  <corinna@vinschen.de>
+
+	Partially revert change from 2006-10-22.  GetSecurityInfo messes up
+	user information on NT4.
+	* sec_helper.cc (security_descriptor::malloc): Drop LocalAlloc
+	considerations.
+	(security_descriptor::realloc): Ditto.
+	(security_descriptor::free): Ditto.
+	* security.cc (get_reg_security): Reinstantiate.
+	(get_nt_object_security): Revert to using NtQuerySecurityObject.
+	* security.h (class security_descriptor): Drop type member.
+	Accommodate throughout.
+	(security_descriptor::size): Constify.
+	(security_descriptor::copy): Ditto.
+
 2006-12-18  Christopher Faylor  <me@cgf.cx>
 
 	* pinfo.cc (set_myself): Use a more foolproof method for determining if
diff --git a/winsup/cygwin/sec_helper.cc b/winsup/cygwin/sec_helper.cc
index 41a9b16d6..7f71c8474 100644
--- a/winsup/cygwin/sec_helper.cc
+++ b/winsup/cygwin/sec_helper.cc
@@ -266,10 +266,7 @@ security_descriptor::malloc (size_t nsize)
 {
   free ();
   if ((psd = (PSECURITY_DESCRIPTOR) ::malloc (nsize)))
-    {
-      sd_size = nsize;
-      type = malloced;
-    }
+    sd_size = nsize;
   return psd;
 }
 
@@ -278,23 +275,9 @@ security_descriptor::realloc (size_t nsize)
 {
   PSECURITY_DESCRIPTOR tmp;
   
-  if (type == malloced)
-    {
-      if (!(tmp = (PSECURITY_DESCRIPTOR) ::realloc (psd, nsize)))
-	return NULL;
-    }
-  else
-    {
-      if (!(tmp = (PSECURITY_DESCRIPTOR) ::malloc (nsize)))
-	return NULL;
-      if (psd)
-	{
-	  memcpy (tmp, psd, LocalSize (psd));
-	  LocalFree (psd);
-	}
-    }
+  if (!(tmp = (PSECURITY_DESCRIPTOR) ::realloc (psd, nsize)))
+    return NULL;
   sd_size = nsize;
-  type = malloced;
   return psd = tmp;
 }
 
@@ -302,15 +285,9 @@ void
 security_descriptor::free ()
 {
   if (psd)
-    {
-      if (type == local_alloced)
-	LocalFree (psd);
-      else
-	::free (psd);
-    }
+    ::free (psd);
   psd = NULL;
   sd_size = 0;
-  type = local_alloced;
 }
 
 #if 0 // unused
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index 3aa658958..33b6876b1 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -1426,21 +1426,76 @@ get_info_from_sd (PSECURITY_DESCRIPTOR psd, mode_t *attribute,
 		  (!acl_exists || !acl)?"NO ":"", *attribute, uid, gid);
 }
 
+static int
+get_reg_security (HANDLE handle, security_descriptor &sd_ret)
+{
+  LONG ret;
+  DWORD len = 0;
+
+  ret = RegGetKeySecurity ((HKEY) handle,
+                           DACL_SECURITY_INFORMATION
+                           | GROUP_SECURITY_INFORMATION
+                           | OWNER_SECURITY_INFORMATION,
+                           sd_ret, &len);
+  if (ret == ERROR_INSUFFICIENT_BUFFER)
+    {       
+      if (!sd_ret.malloc (len))
+        set_errno (ENOMEM);
+      else
+        ret = RegGetKeySecurity ((HKEY) handle,
+                                 DACL_SECURITY_INFORMATION
+                                 | GROUP_SECURITY_INFORMATION
+                                 | OWNER_SECURITY_INFORMATION,
+                                 sd_ret, &len);
+    }
+  if (ret != ERROR_SUCCESS)
+    {
+      __seterrno ();
+      return -1;
+    } 
+  return 0;
+}
+
 int
 get_nt_object_security (HANDLE handle, SE_OBJECT_TYPE object_type,
 			security_descriptor &sd_ret)
 {
-  sd_ret.free ();
-  /* Don't use NtQuerySecurityObject.  It doesn't recognize predefined
-     registry keys. */
-  DWORD ret = GetSecurityInfo (handle, object_type,
-			       DACL_SECURITY_INFORMATION
-			       | GROUP_SECURITY_INFORMATION
-			       | OWNER_SECURITY_INFORMATION,
-			       NULL, NULL, NULL, NULL, sd_ret);
-  if (ret != ERROR_SUCCESS)
-    {
-      __seterrno_from_win_error (ret);
+  NTSTATUS ret;
+  ULONG len = 0;
+  
+  /* Do not try to use GetSecurityInfo (again), unless we drop NT4 support.
+     GetSecurityInfo returns the wrong user information when running in
+     a user session using a token created with NtCreateToken under NT4.
+     Works fine in 2K and above, but that doesn't help a lot. */
+
+  /* Unfortunately, NtQuerySecurityObject doesn't work on predefined registry
+     keys like HKEY_LOCAL_MACHINE.  It fails with "Invalid Handle".  So we
+     have to retreat to the Win32 registry functions for registry keys.
+     What bugs me is that RegGetKeySecurity is obviously just a wrapper
+     around NtQuerySecurityObject, but there seems to be no function to
+     convert pseudo HKEY values to real handles. */
+  if (object_type == SE_REGISTRY_KEY)
+    return get_reg_security (handle, sd_ret);
+        
+  ret = NtQuerySecurityObject (handle,
+                               DACL_SECURITY_INFORMATION
+                               | GROUP_SECURITY_INFORMATION
+                               | OWNER_SECURITY_INFORMATION,
+                               sd_ret, len, &len);
+  if (ret == STATUS_BUFFER_TOO_SMALL)
+    {
+      if (!sd_ret.malloc (len))
+        set_errno (ENOMEM);
+      else
+        ret = NtQuerySecurityObject (handle,
+                                     DACL_SECURITY_INFORMATION
+                                     | GROUP_SECURITY_INFORMATION
+                                     | OWNER_SECURITY_INFORMATION,
+                                     sd_ret, len, &len);
+    }
+  if (ret != STATUS_SUCCESS)
+    {   
+      __seterrno_from_nt_status (ret);
       return -1;
     }
   return 0;
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index cab33be8b..08bcc2f09 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -216,21 +216,16 @@ class security_descriptor {
 protected:
   PSECURITY_DESCRIPTOR psd;
   DWORD sd_size;
-  enum { local_alloced, malloced } type;
 public:
-  security_descriptor () : psd (NULL), sd_size (0), type (local_alloced) {}
+  security_descriptor () : psd (NULL), sd_size (0) {}
   ~security_descriptor () { free (); }
 
   PSECURITY_DESCRIPTOR malloc (size_t nsize);
   PSECURITY_DESCRIPTOR realloc (size_t nsize);
   void free ();
 
-  inline DWORD size () {
-    if (!sd_size && psd && type == local_alloced)
-      sd_size = LocalSize (psd);
-    return sd_size;
-  }
-  inline DWORD copy (void *buf, DWORD buf_size) {
+  inline DWORD size () const { return sd_size; }
+  inline DWORD copy (void *buf, DWORD buf_size) const {
     if (buf_size < size ())
       return sd_size;
     memcpy (buf, psd, sd_size);