Re: [ros-dev] [ros-diffs] [ion] 57284: [NTOSKRNL]: Use the token lock acquire/release macros that were already written instead of manually doing it. Also fix the macros since they didn't work in GCC. No functional change, j...

[Timo Kreuzer]

What was the problem with the old macros? The new ones are error-prone.

if (NeedLock) SepAcquireTokenLockExclusive(Token); // <= fail!

WBR,
Timo




Am 12.09.2012 18:29, schrieb ion(a)svn.reactos.org:
>   Author: ion
>  Date: Wed Sep 12 16:29:28 2012
>  New Revision: 57284
>
>  URL: http://svn.reactos.org/svn/reactos?rev=57284&view=rev
>  Log:
>  [NTOSKRNL]: Use the token lock acquire/release macros that were already written instead
> of manually doing it. Also fix the macros since they didn't work in GCC.
>  No functional change, just code cleanup.
>
>  Modified:
>       trunk/reactos/ntoskrnl/include/internal/se.h
>       trunk/reactos/ntoskrnl/se/access.c
>       trunk/reactos/ntoskrnl/se/semgr.c
>
>  Modified: trunk/reactos/ntoskrnl/include/internal/se.h
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
>  ==============================================================================
>  --- trunk/reactos/ntoskrnl/include/internal/se.h [iso-8859-1] (original)
>  +++ trunk/reactos/ntoskrnl/include/internal/se.h [iso-8859-1] Wed Sep 12 16:29:28 2012
>  @@ -1,4 +1,28 @@
>    #pragma once
>  +
>  +typedef struct _KNOWN_ACE
>  +{
>  +    ACE_HEADER Header;
>  +    ACCESS_MASK Mask;
>  +    ULONG SidStart;
>  +} KNOWN_ACE, *PKNOWN_ACE;
>  +
>  +typedef struct _KNOWN_OBJECT_ACE
>  +{
>  +    ACE_HEADER Header;
>  +    ACCESS_MASK Mask;
>  +    ULONG Flags;
>  +    ULONG SidStart;
>  +} KNOWN_OBJECT_ACE, *PKNOWN_OBJECT_ACE;
>  +
>  +typedef struct _KNOWN_COMPOUND_ACE
>  +{
>  +    ACE_HEADER Header;
>  +    ACCESS_MASK Mask;
>  +    USHORT CompoundAceType;
>  +    USHORT Reserved;
>  +    ULONG SidStart;
>  +} KNOWN_COMPOUND_ACE, *PKNOWN_COMPOUND_ACE;
>    
>    PSID
>    FORCEINLINE
>  @@ -75,6 +99,8 @@
>            return Descriptor->Sacl;
>        }
>    }
>  +
>  +#ifndef RTL_H
>    
>    /* SID Authorities */
>    extern SID_IDENTIFIER_AUTHORITY SeNullSidAuthority;
>  @@ -156,6 +182,19 @@
>    extern PSECURITY_DESCRIPTOR SeSystemDefaultSd;
>    extern PSECURITY_DESCRIPTOR SeUnrestrictedSd;
>    
>  +
>  +#define SepAcquireTokenLockExclusive(Token)                                    \
>  +    KeEnterCriticalRegion();                                                   \
>  +    ExAcquireResourceExclusive(((PTOKEN)Token)->TokenLock, TRUE);              \
>  +
>  +#define SepAcquireTokenLockShared(Token)                                       \
>  +    KeEnterCriticalRegion();                                                   \
>  +    ExAcquireResourceShared(((PTOKEN)Token)->TokenLock, TRUE);                 \
>  +
>  +#define SepReleaseTokenLock(Token)                                             \
>  +    ExReleaseResource(((PTOKEN)Token)->TokenLock);                             \
>  +    KeLeaveCriticalRegion();                                                   \
>  +
>    //
>    // Token Functions
>    //
>  @@ -434,24 +473,6 @@
>        OUT PACCESS_TOKEN* NewToken
>    );
>    
>  -#define SepAcquireTokenLockExclusive(Token)                                    \
>  -  do {                                                                         \
>  -    KeEnterCriticalRegion();                                                   \
>  -    ExAcquireResourceExclusive(((PTOKEN)Token)->TokenLock, TRUE);              \
>  -  while(0)
>  -
>  -#define SepAcquireTokenLockShared(Token)                                       \
>  -  do {                                                                         \
>  -    KeEnterCriticalRegion();                                                   \
>  -    ExAcquireResourceShared(((PTOKEN)Token)->TokenLock, TRUE);                 \
>  -  while(0)
>  -
>  -#define SepReleaseTokenLock(Token)                                             \
>  -  do {                                                                         \
>  -    ExReleaseResource(((PTOKEN)Token)->TokenLock);                             \
>  -    KeLeaveCriticalRegion();                                                   \
>  -  while(0)
>  -
>    VOID NTAPI
>    SeQuerySecurityAccessMask(IN SECURITY_INFORMATION SecurityInformation,
>                              OUT PACCESS_MASK DesiredAccess);
>  @@ -460,4 +481,6 @@
>    SeSetSecurityAccessMask(IN SECURITY_INFORMATION SecurityInformation,
>                            OUT PACCESS_MASK DesiredAccess);
>    
>  +#endif
>  +
>    /* EOF */
>
>  Modified: trunk/reactos/ntoskrnl/se/access.c
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/access.c?rev=5…
>  ==============================================================================
>  --- trunk/reactos/ntoskrnl/se/access.c [iso-8859-1] (original)
>  +++ trunk/reactos/ntoskrnl/se/access.c [iso-8859-1] Wed Sep 12 16:29:28 2012
>  @@ -130,11 +130,7 @@
>        ASSERT(Sid != NULL);
>    
>        /* Lock the token if needed */
>  -    if (!TokenLocked)
>  -    {
>  -        KeEnterCriticalRegion();
>  -        ExAcquireResourceSharedLite(Token->TokenLock, TRUE);
>  -    }
>  +    if (!TokenLocked) SepAcquireTokenLockShared(Token);
>    
>        /* Check if the owner SID is found, handling restricted case as well */
>        Result = SepSidInToken(Token, Sid);
>  @@ -144,11 +140,7 @@
>        }
>    
>        /* Release the lock if we had acquired it */
>  -    if (!TokenLocked)
>  -    {
>  -        ExReleaseResourceLite(Token->TokenLock);
>  -        KeLeaveCriticalRegion();
>  -    }
>  +    if (!TokenLocked) SepReleaseTokenLock(Token);
>    
>        /* Return the result */
>        return Result;
>  @@ -168,15 +160,13 @@
>        TokenControl->TokenSource = Token->TokenSource;
>    
>        /* Lock the token */
>  -    KeEnterCriticalRegion();
>  -    ExAcquireResourceSharedLite(Token->TokenLock, TRUE);
>  +    SepAcquireTokenLockShared(Token);
>    
>        /* Capture the modified it */
>        TokenControl->ModifiedId = Token->ModifiedId;
>    
>        /* Unlock it */
>  -    ExReleaseResourceLite(Token->TokenLock);
>  -    KeLeaveCriticalRegion();
>  +    SepReleaseTokenLock(Token);
>    }
>    
>    NTSTATUS
>  @@ -327,13 +317,11 @@
>        ClientToken = SubjectContext->ClientToken;
>    
>        /* Always lock the primary */
>  -    KeEnterCriticalRegion();
>  -    ExAcquireResourceSharedLite(PrimaryToken->TokenLock, TRUE);
>  +    SepAcquireTokenLockShared(PrimaryToken);
>    
>        /* Lock the impersonation one if it's there */
>        if (!ClientToken) return;
>  -    KeEnterCriticalRegion();
>  -    ExAcquireResourceSharedLite(ClientToken->TokenLock, TRUE);
>  +    SepAcquireTokenLockShared(ClientToken);
>    }
>    
>    /*
>  @@ -351,13 +339,11 @@
>        ClientToken = SubjectContext->ClientToken;
>    
>        /* Always unlock the primary one */
>  -    ExReleaseResourceLite(PrimaryToken->TokenLock);
>  -    KeLeaveCriticalRegion();
>  +    SepReleaseTokenLock(PrimaryToken);
>    
>        /* Unlock the impersonation one if it's there */
>        if (!ClientToken) return;
>  -    ExReleaseResourceLite(ClientToken->TokenLock);
>  -    KeLeaveCriticalRegion();
>  +    SepReleaseTokenLock(ClientToken);
>    }
>    
>    /*
>
>  Modified: trunk/reactos/ntoskrnl/se/semgr.c
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/semgr.c?rev=57…
>  ==============================================================================
>  --- trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] (original)
>  +++ trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] Wed Sep 12 16:29:28 2012
>  @@ -952,8 +952,7 @@
>        SeCaptureSubjectContext(&SubjectSecurityContext);
>    
>        /* Lock the token */
>  -    KeEnterCriticalRegion();
>  -    ExAcquireResourceSharedLite(Token->TokenLock, TRUE);
>  +    SepAcquireTokenLockShared(Token);
>    
>        /* Check if the token is the owner and grant WRITE_DAC and READ_CONTROL rights */
>        if (DesiredAccess & (WRITE_DAC | READ_CONTROL | MAXIMUM_ALLOWED))
>  @@ -990,8 +989,7 @@
>    
>        /* Release subject context and unlock the token */
>        SeReleaseSubjectContext(&SubjectSecurityContext);
>  -    ExReleaseResourceLite(Token->TokenLock);
>  -    KeLeaveCriticalRegion();
>  +    SepReleaseTokenLock(Token);
>    
>        /* Release the captured security descriptor */
>        SeReleaseSecurityDescriptor(CapturedSecurityDescriptor,
>
>
>