Re: Patch, Version 4: Problem solved

[Corinna Vinschen <corinna.vinschen@cityweb.de>]

Corinna Vinschen wrote:
> 
> There's a new problem: In the same code
> 
>         security.cc: set_process_privileges()
> 
> the call to AdjustTokenPrivileges() behave different:
> 
> On setting SE_RESTORE_NAME and SE_BACKUP_NAME rights, GetLastError()
> returns
>         ERROR_SUCCESS in winsup-990303 and
>         ERROR_NOT_ALL_ASSIGNED in winsup-990430.
> As a result, it's impossible for admins, to chown to another user in
> winsup-990430 :-(
> 
> Is anybody willing to help?
> 
> Corinna

I have found the reason:

The sizes and offsets of the structures TOKEN_PRIVILEGES and
LUID_AND_ATTRIBUTES are different between the old and the new defines:

old:	
====
typedef struct _LUID_AND_ATTRIBUTES {
  LUID  Luid;					offset 0, size 8
  DWORD Attributes;				offset 8, size 4
} LUID_AND_ATTRIBUTES;					  size 12

new:
====
typedef struct _LUID_AND_ATTRIBUTES {
        LUID   Luid;				offset 0, size 8
        DWORD  Attributes;			offset 8, size 4
} LUID_AND_ATTRIBUTES;					  size 16!!!

old:
====
typedef struct _TOKEN_PRIVILEGES {
  DWORD PrivilegeCount;				offset 0, size 4
  LUID_AND_ATTRIBUTES Privileges[ANYSIZE_ARRAY];offset 4, size 12
} TOKEN_PRIVILEGES;					  size 16

new:
====
typedef struct _TOKEN_PRIVILEGES {
  DWORD PrivilegeCount;				offset 0, size 4
  LUID_AND_ATTRIBUTES Privileges[ANYSIZE_ARRAY];offset 8, size 16!!!
} TOKEN_PRIVILEGES;					  size 24!!!


This happens, because of the new definition of LARGE_INTEGER, which is
the base type for LUID. It now contains a union member of type
LONGLONG (8 Bytes):

old:
====
typedef struct _LARGE_INTEGER {
  DWORD LowPart;
  LONG  HighPart;
} LARGE_INTEGER;

new:
====
typedef union _LARGE_INTEGER {
        struct {
                DWORD LowPart;
                LONG HighPart;
        }_STRUCT_NAME(u);
        LONGLONG QuadPart;
} LARGE_INTEGER;

So gcc alignes the LUID type to a 8 byte boundary, instead of a
4 byte boundary.
For testing purposes, I have made the "LONGLONG QuadPart" to a comment
and now, AdjustTokenPrivileges() works as expected.

So I like to suggest, to erase the definition of QuadPart in the
definitions of LARGE_INTEGER and ULARGE_INTEGER in include/winnt.h,
until the alignment problem is solved. The fix is following.
Moreover, I have attached a tiny fix to my security patch, which
solves a "permission denied" problem, if you set file permissions
to e.g. "chmod 000 foo".

Regards,
Corinna

ChangeLog:
==========

Sun May  2 23:50:00  Corinna Vinschen  <corinna.vinschen@cityweb.de>

	* include/winnt.h: Temporary erased definitions of QuadPart
	in LARGE_INTEGER and ULARGE_INTEGER.
	* security.cc (set_nt_attribute): Set standard attributes so
	that reading and writing attributes for user and administrators
	isn't hindered.

Index: include/winnt.h
===================================================================
RCS file: /src/cvsroot/winsup-990430/include/winnt.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 winnt.h
--- include/winnt.h     1999/05/01 21:52:53     1.1.1.1
+++ include/winnt.h     1999/05/02 21:42:05
@@ -939,14 +939,14 @@ typedef union _LARGE_INTEGER {
                DWORD LowPart;
                LONG HighPart;
        }_STRUCT_NAME(u);
-       LONGLONG QuadPart;
+       //LONGLONG QuadPart;
 } LARGE_INTEGER,*PLARGE_INTEGER;
 typedef union _ULARGE_INTEGER {
        struct {
                DWORD LowPart;
                DWORD HighPart;
        }_STRUCT_NAME(u);
-       DWORDLONG QuadPart;
+       //DWORDLONG QuadPart;
 } ULARGE_INTEGER,*PULARGE_INTEGER;
 typedef LARGE_INTEGER LUID,*PLUID;
 typedef struct _LUID_AND_ATTRIBUTES {
Index: security.cc
===================================================================
RCS file: /src/cvsroot/winsup-990430/security.cc,v
retrieving revision 1.2
diff -u -p -r1.2 security.cc
--- security.cc 1999/05/02 08:58:46     1.2
+++ security.cc 1999/05/02 21:48:35
@@ -644,13 +644,12 @@ set_nt_attribute (const char *file, uid_

       if (sidGroup)
        {
-         access = 0;
+          access = STANDARD_RIGHTS_READ;
+         if (EqualSid (sidGroup, get_admin_sid ()))
+           access |= WRITE_DAC | WRITE_OWNER
+                     | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
          if (attribute & S_IRGRP)
-           {
-             access |= FILE_GENERIC_READ;
-             if (EqualSid (sidGroup, get_admin_sid ()))
-               access |= FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
-           }
+           access |= FILE_GENERIC_READ;
          if (attribute & S_IWGRP)
            access |= STANDARD_RIGHTS_ALL | FILE_GENERIC_WRITE
                      | DELETE | FILE_DELETE_CHILD;
@@ -662,7 +661,7 @@ set_nt_attribute (const char *file, uid_
            ace->Header.AceFlags |= OBJECT_INHERIT_ACE |
CONTAINER_INHERIT_ACE;
        }

-      access = 0;
+      access = STANDARD_RIGHTS_READ;
       if (attribute & S_IROTH)
        access |= FILE_GENERIC_READ;
       if (attribute & S_IWOTH)