- Rename obwait/obsecure

- Check for valid access in NtSignalAndWaitForSingleObject and don't check if ResultLength isn't NULL in NtQueryObject, because it's not optional.
- Add Thomas's name to a file where it wasn't.
- Reformatting (only code change is #2 above)

svn path=/trunk/; revision=22494

reactos/ntoskrnl/ob/obsecure.c

@@ -1,7 +1,7 @@
 /*
 * PROJECT:         ReactOS Kernel
 * LICENSE:         GPL - See COPYING in the top level directory
-* FILE:            ntoskrnl/ob/security.c
+* FILE:            ntoskrnl/ob/obsecure.c
 * PURPOSE:         SRM Interface of the Object Manager
 * PROGRAMMERS:     Alex Ionescu (alex@relsoft.net)
 *                  Eric Kohl
@@ -13,6 +13,8 @@
 #define NDEBUG
 #include <internal/debug.h>
 
+#define TAG_SEC_QUERY   TAG('O', 'b', 'S', 'q')
+
 /* FUNCTIONS ***************************************************************/
 
 /*++
@@ -144,7 +146,7 @@ ObGetObjectSecurity(IN PVOID Object,
     /* Allocate security descriptor */
     *SecurityDescriptor = ExAllocatePoolWithTag(PagedPool,
                                                 Length,
-                                                TAG('O', 'b', 'S', 'q'));
+                                                TAG_SEC_QUERY);
     if (!(*SecurityDescriptor)) return STATUS_INSUFFICIENT_RESOURCES;
 
     /* Query security descriptor */
@@ -248,72 +250,75 @@ NtQuerySecurityObject(IN HANDLE Handle,
     KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
     PVOID Object;
     POBJECT_HEADER Header;
-    ACCESS_MASK DesiredAccess = (ACCESS_MASK)0;
+    POBJECT_TYPE Type;
+    ACCESS_MASK DesiredAccess;
     NTSTATUS Status = STATUS_SUCCESS;
     PAGED_CODE();
 
+    /* Check if we came from user mode */
     if (PreviousMode != KernelMode)
     {
+        /* Enter SEH */
         _SEH_TRY
         {
+            /* Probe the SD and the length pointer */
             ProbeForWrite(SecurityDescriptor, Length, sizeof(ULONG));
-            if (ResultLength != NULL)
-            {
             ProbeForWriteUlong(ResultLength);
         }
-        }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
         }
         _SEH_END;
 
+        /* Fail if we got an access violation */
         if (!NT_SUCCESS(Status)) return Status;
     }
 
-    /* get the required access rights for the operation */
+    /* Get the required access rights for the operation */
-    SeQuerySecurityAccessMask(SecurityInformation,
+    SeQuerySecurityAccessMask(SecurityInformation, &DesiredAccess);
-                              &DesiredAccess);
 
+    /* Reference the object */
     Status = ObReferenceObjectByHandle(Handle,
                                        DesiredAccess,
                                        NULL,
                                        PreviousMode,
                                        &Object,
                                        NULL);
+    if (!NT_SUCCESS(Status)) return Status;
 
-    if (NT_SUCCESS(Status))
+    /* Get the Object Header and Type */
-    {
     Header = OBJECT_TO_OBJECT_HEADER(Object);
-        ASSERT(Header->Type != NULL);
+    Type = Header->Type;
 
-        Status = Header->Type->TypeInfo.SecurityProcedure(
+    /* Call the security procedure's query function */
-            Object,
+    Status = Type->TypeInfo.SecurityProcedure(Object,
                                               QuerySecurityDescriptor,
                                               SecurityInformation,
                                               SecurityDescriptor,
                                               &Length,
                                               &Header->SecurityDescriptor,
-            Header->Type->TypeInfo.PoolType,
+                                              Type->TypeInfo.PoolType,
-            &Header->Type->TypeInfo.GenericMapping);
+                                              &Type->TypeInfo.GenericMapping);
 
+    /* Dereference the object */
     ObDereferenceObject(Object);
 
-        /* return the required length */
+    /* Protect write with SEH */
-        if (ResultLength != NULL)
-        {
     _SEH_TRY
     {
+        /* Return the needed length */
         *ResultLength = Length;
     }
     _SEH_EXCEPT(_SEH_ExSystemExceptionFilter)
     {
+        /* Get the exception code */
         Status = _SEH_GetExceptionCode();
     }
     _SEH_END;
-    }
-    }
 
+    /* Return status */
     return Status;
 }
 
@@ -346,140 +351,78 @@ NtSetSecurityObject(IN HANDLE Handle,
     KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
     PVOID Object;
     POBJECT_HEADER Header;
-    SECURITY_DESCRIPTOR_RELATIVE *CapturedSecurityDescriptor;
+    POBJECT_TYPE Type;
-    ACCESS_MASK DesiredAccess = (ACCESS_MASK)0;
+    SECURITY_DESCRIPTOR_RELATIVE *CapturedDescriptor;
+    ACCESS_MASK DesiredAccess;
     NTSTATUS Status;
     PAGED_CODE();
 
-    /* make sure the caller doesn't pass a NULL security descriptor! */
+    /* Make sure the caller doesn't pass a NULL security descriptor! */
-    if (SecurityDescriptor == NULL) return STATUS_ACCESS_DENIED;
+    if (!SecurityDescriptor) return STATUS_ACCESS_VIOLATION;
 
-    /* capture and make a copy of the security descriptor */
+    /* Capture and make a copy of the security descriptor */
     Status = SeCaptureSecurityDescriptor(SecurityDescriptor,
                                          PreviousMode,
                                          PagedPool,
                                          TRUE,
                                          (PSECURITY_DESCRIPTOR*)
-                                         &CapturedSecurityDescriptor);
+                                         &CapturedDescriptor);
-    if (!NT_SUCCESS(Status))
+    if (!NT_SUCCESS(Status)) return Status;
-    {
-        DPRINT1("Capturing the security descriptor failed! Status: 0x%lx\n", Status);
-        return Status;
-    }
 
     /*
-     * make sure the security descriptor passed by the caller
+     * Make sure the security descriptor passed by the caller
      * is valid for the operation we're about to perform
      */
     if (((SecurityInformation & OWNER_SECURITY_INFORMATION) &&
-         (CapturedSecurityDescriptor->Owner == 0)) ||
+         !(CapturedDescriptor->Owner)) ||
         ((SecurityInformation & GROUP_SECURITY_INFORMATION) &&
-         (CapturedSecurityDescriptor->Group == 0)))
+         !(CapturedDescriptor->Group)))
     {
+        /* Set the failure status */
         Status = STATUS_INVALID_SECURITY_DESCR;
     }
     else
     {
-        /* get the required access rights for the operation */
+        /* Set the required access rights for the operation */
-        SeSetSecurityAccessMask(SecurityInformation,
+        SeSetSecurityAccessMask(SecurityInformation, &DesiredAccess);
-                                &DesiredAccess);
 
+        /* Reference the object */
         Status = ObReferenceObjectByHandle(Handle,
                                            DesiredAccess,
                                            NULL,
                                            PreviousMode,
                                            &Object,
                                            NULL);
-
         if (NT_SUCCESS(Status))
         {
+            /* Get the Object Header and Type */
             Header = OBJECT_TO_OBJECT_HEADER(Object);
-            ASSERT(Header->Type != NULL);
+            Type = Header->Type;
 
-            Status = Header->Type->TypeInfo.SecurityProcedure(
+            /* Call the security procedure's set function */
-                Object,
+            Status = Type->TypeInfo.SecurityProcedure(Object,
                                                       SetSecurityDescriptor,
                                                       SecurityInformation,
-                (PSECURITY_DESCRIPTOR)SecurityDescriptor,
+                                                      SecurityDescriptor,
                                                       NULL,
-                &Header->SecurityDescriptor,
+                                                      &Header->
-                Header->Type->TypeInfo.PoolType,
+                                                      SecurityDescriptor,
-                &Header->Type->TypeInfo.GenericMapping);
+                                                      Type->TypeInfo.PoolType,
+                                                      &Type->
+                                                      TypeInfo.GenericMapping);
 
+            /* Now we can dereference the object */
             ObDereferenceObject(Object);
         }
     }
 
-    /* release the descriptor */
+    /* Release the descriptor and return status */
-    SeReleaseSecurityDescriptor((PSECURITY_DESCRIPTOR)CapturedSecurityDescriptor,
+    SeReleaseSecurityDescriptor((PSECURITY_DESCRIPTOR)CapturedDescriptor,
                                 PreviousMode,
                                 TRUE);
-
     return Status;
 }
 
-/*++
-* @name ObLogSecurityDescriptor
-* @unimplemented NT5.2
-*
-*     The ObLogSecurityDescriptor routine <FILLMEIN>
-*
-* @param InputSecurityDescriptor
-*        <FILLMEIN>
-*
-* @param OutputSecurityDescriptor
-*        <FILLMEIN>
-*
-* @param RefBias
-*        <FILLMEIN>
-*
-* @return STATUS_SUCCESS or appropriate error value.
-*
-* @remarks None.
-*
-*--*/
-NTSTATUS
-NTAPI
-ObLogSecurityDescriptor(IN PSECURITY_DESCRIPTOR InputSecurityDescriptor,
-                        OUT PSECURITY_DESCRIPTOR *OutputSecurityDescriptor,
-                        IN ULONG RefBias)
-{
-    /* HACK: Return the same descriptor back */
-    PISECURITY_DESCRIPTOR SdCopy;
-    DPRINT1("ObLogSecurityDescriptor is not implemented!\n",
-            InputSecurityDescriptor);
-
-    SdCopy = ExAllocatePool(PagedPool, sizeof(*SdCopy));
-    RtlMoveMemory(SdCopy, InputSecurityDescriptor, sizeof(*SdCopy));
-    *OutputSecurityDescriptor = SdCopy;
-    return STATUS_SUCCESS;
-}
-
-/*++
-* @name ObDereferenceSecurityDescriptor
-* @unimplemented NT5.2
-*
-*     The ObDereferenceSecurityDescriptor routine <FILLMEIN>
-*
-* @param SecurityDescriptor
-*        <FILLMEIN>
-*
-* @param Count
-*        <FILLMEIN>
-*
-* @return STATUS_SUCCESS or appropriate error value.
-*
-* @remarks None.
-*
-*--*/
-VOID
-NTAPI
-ObDereferenceSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
-                                IN ULONG Count)
-{
-    DPRINT1("ObDereferenceSecurityDescriptor is not implemented!\n");
-}
-
 /*++
 * @name ObQueryObjectAuditingByHandle
 * @implemented NT5
@@ -545,4 +488,66 @@ ObQueryObjectAuditingByHandle(IN HANDLE Handle,
     return Status;
 }
 
+/*++
+* @name ObLogSecurityDescriptor
+* @unimplemented NT5.2
+*
+*     The ObLogSecurityDescriptor routine <FILLMEIN>
+*
+* @param InputSecurityDescriptor
+*        <FILLMEIN>
+*
+* @param OutputSecurityDescriptor
+*        <FILLMEIN>
+*
+* @param RefBias
+*        <FILLMEIN>
+*
+* @return STATUS_SUCCESS or appropriate error value.
+*
+* @remarks None.
+*
+*--*/
+NTSTATUS
+NTAPI
+ObLogSecurityDescriptor(IN PSECURITY_DESCRIPTOR InputSecurityDescriptor,
+                        OUT PSECURITY_DESCRIPTOR *OutputSecurityDescriptor,
+                        IN ULONG RefBias)
+{
+    /* HACK: Return the same descriptor back */
+    PISECURITY_DESCRIPTOR SdCopy;
+    DPRINT1("ObLogSecurityDescriptor is not implemented!\n",
+            InputSecurityDescriptor);
+
+    SdCopy = ExAllocatePool(PagedPool, sizeof(*SdCopy));
+    RtlMoveMemory(SdCopy, InputSecurityDescriptor, sizeof(*SdCopy));
+    *OutputSecurityDescriptor = SdCopy;
+    return STATUS_SUCCESS;
+}
+
+/*++
+* @name ObDereferenceSecurityDescriptor
+* @unimplemented NT5.2
+*
+*     The ObDereferenceSecurityDescriptor routine <FILLMEIN>
+*
+* @param SecurityDescriptor
+*        <FILLMEIN>
+*
+* @param Count
+*        <FILLMEIN>
+*
+* @return STATUS_SUCCESS or appropriate error value.
+*
+* @remarks None.
+*
+*--*/
+VOID
+NTAPI
+ObDereferenceSecurityDescriptor(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
+                                IN ULONG Count)
+{
+    DPRINT1("ObDereferenceSecurityDescriptor is not implemented!\n");
+}
+
 /* EOF */

reactos/ntoskrnl/ob/obwait.c

@@ -1,10 +1,10 @@
 /*
  * PROJECT:         ReactOS Kernel
  * LICENSE:         GPL - See COPYING in the top level directory
- * FILE:            ntoskrnl/ob/wait.c
+ * FILE:            ntoskrnl/ob/obwait.c
  * PURPOSE:         Handles Waiting on Objects
  * PROGRAMMERS:     Alex Ionescu (alex@relsoft.net)
- *                  David Welch (welch@mcmail.com)
+ *                  Thomas Weidenmueller (w3seek@reactos.org)
  */
 
 /* INCLUDES ******************************************************************/
@@ -65,7 +65,6 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     ACCESS_MASK GrantedAccess;
     PVOID DefaultObject;
     NTSTATUS Status = STATUS_SUCCESS;
-
     DPRINT("NtWaitForMultipleObjects(ObjectCount %lu HandleArray[] %x,"
            " Alertable %d, TimeOut %x)\n",
            ObjectCount, HandleArray, Alertable, TimeOut);
@@ -78,7 +77,6 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     if ((ObjectCount > MAXIMUM_WAIT_OBJECTS) || !ObjectCount)
     {
         Status = STATUS_INVALID_PARAMETER_1;
-        DPRINT1("No object count, or too many objects\n");
         goto Quickie;
     }
 
@@ -86,20 +84,20 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     if ((WaitType != WaitAll) && (WaitType != WaitAny))
     {
         Status = STATUS_INVALID_PARAMETER_3;
-        DPRINT1("Invalid wait type\n");
         goto Quickie;
     }
 
-    /* Capture arguments */
+    /* Enter SEH */
     _SEH_TRY
     {
+        /* Check if the call came from user mode */
         if(PreviousMode != KernelMode)
         {
+            /* Probe all the handles */
             ProbeForRead(HandleArray,
                          ObjectCount * sizeof(HANDLE),
                          sizeof(HANDLE));
-            
+            if (TimeOut)
-            if(TimeOut)
             {
                 /* Make a local copy of the timeout on the stack */
                 SafeTimeOut = ProbeForReadLargeInteger(TimeOut);
@@ -118,18 +116,21 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     }
     _SEH_HANDLE
     {
+        /* Get exception code */
         Status = _SEH_GetExceptionCode();
     }
     _SEH_END;
 
-    if(!NT_SUCCESS(Status)) goto Quickie;
+    /* Fail if we raised an exception */
+    if (!NT_SUCCESS(Status)) goto Quickie;
 
     /* Check if we can use the internal Wait Array */
     if (ObjectCount > THREAD_WAIT_OBJECTS)
     {
         /* Allocate from Pool */
         WaitBlockArray = ExAllocatePoolWithTag(NonPagedPool,
-                                               ObjectCount * sizeof(KWAIT_BLOCK),
+                                               ObjectCount *
+                                               sizeof(KWAIT_BLOCK),
                                                TAG_WAIT);
     }
 
@@ -137,7 +138,7 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     do
     {
         /* Use the right Executive Handle */
-        if(ObIsKernelHandle(Handles[i], PreviousMode))
+        if (ObIsKernelHandle(Handles[i], PreviousMode))
         {
             /* Use the System Handle Table and decode */
             HandleTable = ObpKernelHandleTable;
@@ -164,7 +165,6 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
             /* Unlock the entry and fail */
             ExUnlockHandleTableEntry(HandleTable, HandleEntry);
             Status = STATUS_ACCESS_DENIED;
-            DPRINT1("Handle doesn't have SYNCH access\n");
             goto Quickie;
         }
 
@@ -218,7 +218,6 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
                 {
                     /* Fail */
                     Status = STATUS_INVALID_PARAMETER_MIX;
-                    DPRINT1("Objects duplicated with WaitAll\n");
                     goto Quickie;
                 }
             }
@@ -247,6 +246,7 @@ NtWaitForMultipleObjects(IN ULONG ObjectCount,
     }
     _SEH_HANDLE
     {
+        /* Get the exception code */
         Status = _SEH_GetExceptionCode();
     }
     _SEH_END;
@@ -255,9 +255,13 @@ Quickie:
     /* First derefence */
     while (ReferencedObjects)
     {
+        /* Decrease the number of objects */
         ReferencedObjects--;
+
+        /* Check if we had a valid object in this position */
         if (Objects[ReferencedObjects])
         {
+            /* Dereference it */
             ObDereferenceObject(Objects[ReferencedObjects]);
         }
     }
@@ -269,7 +273,6 @@ Quickie:
     if (LockInUse) KeLeaveCriticalRegion();
 
     /* Return status */
-    DPRINT("Returning: %x\n", Status);
     return Status;
 }
 
@@ -303,12 +306,11 @@ NtWaitForSingleObject(IN HANDLE ObjectHandle,
     KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
     LARGE_INTEGER SafeTimeOut;
     NTSTATUS Status = STATUS_SUCCESS;
-
     DPRINT("NtWaitForSingleObject(ObjectHandle %x, Alertable %d, TimeOut %x)\n",
             ObjectHandle,Alertable,TimeOut);
 
-    /* Capture timeout */
+    /* Check if we came with a timeout from user mode */
-    if(TimeOut && PreviousMode != KernelMode)
+    if (TimeOut && PreviousMode != KernelMode)
     {
         _SEH_TRY
         {
@@ -318,11 +320,13 @@ NtWaitForSingleObject(IN HANDLE ObjectHandle,
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
         }
         _SEH_END;
 
-        if(!NT_SUCCESS(Status)) return Status;
+        /* Fail if we got an access violation */
+        if (!NT_SUCCESS(Status)) return Status;
     }
 
     /* Get the Object */
@@ -345,9 +349,10 @@ NtWaitForSingleObject(IN HANDLE ObjectHandle,
                                      (ULONG_PTR)WaitableObject);
         }
 
-        /* Now wait. Also SEH this since it can also raise an exception */
+        /* SEH this since it can also raise an exception */
         _SEH_TRY
         {
+            /* Ask the kernel to do the wait */
             Status = KeWaitForSingleObject(WaitableObject,
                                            UserRequest,
                                            PreviousMode,
@@ -356,6 +361,7 @@ NtWaitForSingleObject(IN HANDLE ObjectHandle,
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
         }
         _SEH_END;
@@ -407,9 +413,8 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
     OBJECT_HANDLE_INFORMATION HandleInfo;
     NTSTATUS Status = STATUS_SUCCESS;
 
-    /* Capture timeout */
+    /* Check if we came with a timeout from user mode */
-    DPRINT("NtSignalAndWaitForSingleObject\n");
+    if (TimeOut && PreviousMode != KernelMode)
-    if(TimeOut && PreviousMode != KernelMode)
     {
         _SEH_TRY
         {
@@ -419,11 +424,13 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
         }
         _SEH_END;
 
-        if(!NT_SUCCESS(Status)) return Status;
+        /* Fail if we got an access violation */
+        if (!NT_SUCCESS(Status)) return Status;
     }
 
     /* Start by getting the signal object*/
@@ -433,10 +440,7 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
                                        PreviousMode,
                                        &SignalObj,
                                        &HandleInfo);
-    if (!NT_SUCCESS(Status))
+    if (!NT_SUCCESS(Status)) return Status;
-    {
-        return Status;
-    }
 
     /* Now get the wait object */
     Status = ObReferenceObjectByHandle(WaitableObjectHandle,
@@ -447,6 +451,7 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
                                        NULL);
     if (!NT_SUCCESS(Status))
     {
+        /* Failed to reference the wait object */
         ObDereferenceObject(SignalObj);
         return Status;
     }
@@ -466,49 +471,68 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
     Type = OBJECT_TO_OBJECT_HEADER(WaitObj)->Type;
     if (Type == ExEventObjectType)
     {
+        /* Check if we came from user-mode without the right access */
+        if ((PreviousMode != KernelMode) &&
+            !(HandleInfo.GrantedAccess & EVENT_MODIFY_STATE))
+        {
+            /* Fail: lack of rights */
+            goto Quickie;
+        }
+
         /* Set the Event */
-        /* FIXME: Check permissions */
         KeSetEvent(SignalObj, EVENT_INCREMENT, TRUE);
     }
     else if (Type == ExMutantObjectType)
     {
-        /* Release the Mutant. This can raise an exception*/
+        /* This can raise an exception */
         _SEH_TRY
         {
+            /* Release the mutant */
             KeReleaseMutant(SignalObj, MUTANT_INCREMENT, FALSE, TRUE);
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
-            goto Quickie;
         }
         _SEH_END;
     }
     else if (Type == ExSemaphoreObjectType)
     {
-        /* Release the Semaphore. This can raise an exception*/
+        /* Check if we came from user-mode without the right access */
-        /* FIXME: Check permissions */
+        if ((PreviousMode != KernelMode) &&
+            !(HandleInfo.GrantedAccess & SEMAPHORE_MODIFY_STATE))
+        {
+            /* Fail: lack of rights */
+            goto Quickie;
+        }
+
+        /* This can raise an exception*/
         _SEH_TRY
         {
+            /* Release the semaphore */
             KeReleaseSemaphore(SignalObj, SEMAPHORE_INCREMENT, 1, TRUE);
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
-            goto Quickie;
         }
         _SEH_END;
     }
     else
     {
+        /* This isn't a valid object to be waiting on */
         Status = STATUS_OBJECT_TYPE_MISMATCH;
-        DPRINT1("Waiting on invalid object type\n");
-        goto Quickie;
     }
 
-    /* Now wait. Also SEH this since it can also raise an exception */
+    /* Make sure we didn't fail */
+    if (NT_SUCCESS(Status))
+    {
+        /* SEH this since it can also raise an exception */
         _SEH_TRY
         {
+            /* Perform the wait now */
             Status = KeWaitForSingleObject(WaitableObject,
                                            UserRequest,
                                            PreviousMode,
@@ -517,11 +541,13 @@ NtSignalAndWaitForSingleObject(IN HANDLE ObjectHandleToSignal,
         }
         _SEH_HANDLE
         {
+            /* Get the exception code */
             Status = _SEH_GetExceptionCode();
         }
         _SEH_END;
+    }
 
-    /* We're done here */
+    /* We're done here, dereference both objects */
 Quickie:
     ObDereferenceObject(SignalObj);
     ObDereferenceObject(WaitObj);