[ros-diffs] [ion] 22851: - Add locking in IopDeleteFile, and only check for completion context or name if the file has a DO, otherwise those shouldn't exist. - Added support for device objects as well in IopSecurityFile, since the code is similar. Also fixed sync semantics for file objects, and made other little small fixes.

Author: ion
Date: Wed Jul  5 02:28:23 2006
New Revision: 22851

URL: http://svn.reactos.org/svn/reactos?rev=22851&view=rev
Log:
- Add locking in IopDeleteFile, and only check for completion context or name if the file
has a DO, otherwise those shouldn't exist.
- Added support for device objects as well in IopSecurityFile, since the code is similar.
Also fixed sync semantics for file objects, and made other little small fixes.

Modified:
    trunk/reactos/ntoskrnl/include/internal/io.h
    trunk/reactos/ntoskrnl/io/iomgr/file.c
    trunk/reactos/ntoskrnl/io/iomgr/iofunc.c
    trunk/reactos/ntoskrnl/io/iomgr/irp.c

Modified: trunk/reactos/ntoskrnl/include/internal/io.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/io.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/io.h Wed Jul  5 02:28:23 2006
@@ -620,6 +620,23 @@
 );
 
 //
+// IRP Routines
+//
+NTSTATUS
+NTAPI
+IopCleanupFailedIrp(
+    IN PFILE_OBJECT FileObject,
+    IN PKEVENT EventObject
+);
+
+VOID
+NTAPI
+IopAbortInterruptedIrp(
+    IN PKEVENT EventObject,
+    IN PIRP Irp
+);
+
+//
 // Shutdown routines
 //
 VOID

Modified: trunk/reactos/ntoskrnl/io/iomgr/file.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/file.c?r…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/file.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/file.c Wed Jul  5 02:28:23 2006
@@ -163,8 +163,8 @@
 }
 
 VOID
-STDCALL
-IopDeleteFile(PVOID ObjectBody)
+NTAPI
+IopDeleteFile(IN PVOID ObjectBody)
 {
     PFILE_OBJECT FileObject = (PFILE_OBJECT)ObjectBody;
     PIRP Irp;
@@ -173,18 +173,26 @@
     KEVENT Event;
     PDEVICE_OBJECT DeviceObject;
 
-    DPRINT("IopDeleteFile()\n");
-
+    /* Check if the file has a device object */
     if (FileObject->DeviceObject)
     {
         /* Check if this is a direct open or not */
         if (FileObject->Flags & FO_DIRECT_DEVICE_OPEN)
         {
+            /* Get the attached device */
             DeviceObject = IoGetAttachedDevice(FileObject->DeviceObject);
         }
         else
         {
+            /* Use the file object's device object */
             DeviceObject = IoGetRelatedDeviceObject(FileObject);
+        }
+
+        /* Check if this file was opened for Synch I/O */
+        if (FileObject->Flags & FO_SYNCHRONOUS_IO)
+        {
+            /* Lock it */
+            IopLockFileObject(FileObject);
         }
 
         /* Clear and set up Events */
@@ -193,6 +201,7 @@
 
         /* Allocate an IRP */
         Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE);
+        if (!Irp) return;
 
         /* Set it up */
         Irp->UserEvent = &Event;
@@ -207,104 +216,129 @@
         StackPtr->DeviceObject = DeviceObject;
         StackPtr->FileObject = FileObject;
 
+        /* Queue the IRP */
+        //IopQueueIrpToThread(Irp);
+
         /* Call the FS Driver */
         Status = IoCallDriver(DeviceObject, Irp);
-
-        /* Wait for completion */
         if (Status == STATUS_PENDING)
         {
+            /* Wait for completion */
             KeWaitForSingleObject(&Event, Executive, KernelMode, FALSE, NULL);
         }
+
+        /* Free the IRP */
         IoFreeIrp(Irp);
 
-    }
-
-    /* Clear the file name */
-    if (FileObject->FileName.Buffer)
-    {
-       ExFreePool(FileObject->FileName.Buffer);
-       FileObject->FileName.Buffer = NULL;
-    }
-
-    /* Free the completion context */
-    if (FileObject->CompletionContext)
-    {
-       ObDereferenceObject(FileObject->CompletionContext->Port);
-       ExFreePool(FileObject->CompletionContext);
-    }
-}
-
-NTSTATUS
-STDCALL
-IopSecurityFile(PVOID ObjectBody,
-                SECURITY_OPERATION_CODE OperationCode,
-                SECURITY_INFORMATION SecurityInformation,
-                PSECURITY_DESCRIPTOR SecurityDescriptor,
-                PULONG BufferLength,
-                PSECURITY_DESCRIPTOR *OldSecurityDescriptor,    
-                POOL_TYPE PoolType,
-                PGENERIC_MAPPING GenericMapping)
+        /* Clear the file name */
+        if (FileObject->FileName.Buffer)
+        {
+           ExFreePool(FileObject->FileName.Buffer);
+           FileObject->FileName.Buffer = NULL;
+        }
+
+        /* Check if the FO had a completion port */
+        if (FileObject->CompletionContext)
+        {
+            /* Free it */
+            ObDereferenceObject(FileObject->CompletionContext->Port);
+            ExFreePool(FileObject->CompletionContext);
+        }
+    }
+}
+
+NTSTATUS
+NTAPI
+IopSecurityFile(IN PVOID ObjectBody,
+                IN SECURITY_OPERATION_CODE OperationCode,
+                IN SECURITY_INFORMATION SecurityInformation,
+                IN PSECURITY_DESCRIPTOR SecurityDescriptor,
+                IN OUT PULONG BufferLength,
+                IN OUT PSECURITY_DESCRIPTOR *OldSecurityDescriptor,
+                IN POOL_TYPE PoolType,
+                IN OUT PGENERIC_MAPPING GenericMapping)
 {
     IO_STATUS_BLOCK IoStatusBlock;
     PIO_STACK_LOCATION StackPtr;
     PFILE_OBJECT FileObject;
     PDEVICE_OBJECT DeviceObject;
-    ULONG MajorFunction;
     PIRP Irp;
     BOOLEAN LocalEvent = FALSE;
     KEVENT Event;
     NTSTATUS Status = STATUS_SUCCESS;
-
-    DPRINT("IopSecurityFile() called\n");
-
-    FileObject = (PFILE_OBJECT)ObjectBody;
-
-    if (OperationCode == QuerySecurityDescriptor)
-    {
-        MajorFunction = IRP_MJ_QUERY_SECURITY;
-        DPRINT("Query security descriptor\n");
+    PAGED_CODE();
+
+    /* Check if this is a device or file */
+    if (((PFILE_OBJECT)ObjectBody)->Type == IO_TYPE_DEVICE)
+    {
+        /* It's a device */
+        DeviceObject = (PDEVICE_OBJECT)ObjectBody;
+        FileObject = NULL;
+    }
+    else
+    {
+        /* It's a file */
+        FileObject = (PFILE_OBJECT)ObjectBody;
+
+        /* Check if this is a direct open */
+        if (FileObject->Flags & FO_DIRECT_DEVICE_OPEN)
+        {
+            /* Get the Device Object */
+            DeviceObject = IoGetAttachedDevice(FileObject->DeviceObject);
+        }
+        else
+        {
+            /* Otherwise, use the direct device*/
+            DeviceObject = FileObject->DeviceObject;
+        }
+    }
+
+    /* Check if the request was for a device object */
+    if (!(FileObject) || (FileObject->Flags & FO_DIRECT_DEVICE_OPEN))
+    {
+        /* Check what kind of request this was */
+        if (OperationCode == QuerySecurityDescriptor)
+        {
+            DPRINT1("FIXME: Device Query security descriptor UNHANDLED\n");
+            return STATUS_SUCCESS;
+        }
+        else if (OperationCode == DeleteSecurityDescriptor)
+        {
+            /* Simply return success */
+            return STATUS_SUCCESS;
+        }
+        else if (OperationCode == AssignSecurityDescriptor)
+        {
+            /* Make absolutely sure this is a device object */
+            if (!(FileObject) || !(FileObject->Flags & FO_STREAM_FILE))
+            {
+                /* Assign the Security Descriptor */
+                DeviceObject->SecurityDescriptor = SecurityDescriptor;
+            }
+
+            /* Return success */
+            return STATUS_SUCCESS;
+        }
+        else
+        {
+            DPRINT1("FIXME: Set SD unimplemented for Devices\n");
+            return STATUS_SUCCESS;
+        }
     }
     else if (OperationCode == DeleteSecurityDescriptor)
     {
-        DPRINT("Delete\n");
+        /* Same as for devices, do nothing */
         return STATUS_SUCCESS;
     }
-    else if (OperationCode == AssignSecurityDescriptor)
-    {
-        /* If this is a direct open, we can assign it */
-        if (FileObject->Flags & FO_DIRECT_DEVICE_OPEN)
-        {
-            /* Get the Device Object */
-            DPRINT("here\n");
-            DeviceObject = IoGetAttachedDevice(FileObject->DeviceObject);
-
-            /* Assign the Security Descriptor */
-            DeviceObject->SecurityDescriptor = SecurityDescriptor;
-        }
-        return STATUS_SUCCESS;
-    }
-    else
-    {
-        MajorFunction = IRP_MJ_SET_SECURITY;
-        DPRINT("Set security descriptor\n");
-
-        /* If this is a direct open, we can set it */
-        if (FileObject->Flags & FO_DIRECT_DEVICE_OPEN)
-        {
-            DPRINT1("Set SD unimplemented for Devices\n");
-            return STATUS_SUCCESS;
-        }
-    }
-
-    /* Get the Device Object */
-    DPRINT1("FileObject: %p\n", FileObject);
-    DeviceObject = IoGetRelatedDeviceObject(FileObject);
+
+    /* At this point, we know we're a file. Reference it */
+    ObReferenceObject(FileObject);
 
     /* Check if we should use Sync IO or not */
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
-        /* Use File Object event */
-        KeClearEvent(&FileObject->Event);
+        /* Lock the file object */
+        IopLockFileObject(FileObject);
     }
     else
     {
@@ -313,65 +347,95 @@
         LocalEvent = TRUE;
     }
 
+    /* Clear the File Object event */
+    KeClearEvent(&FileObject->Event);
+
+    /* Get the device object */
+    DeviceObject = IoGetRelatedDeviceObject(FileObject);
+
     /* Allocate the IRP */
     Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE);
+    if (!Irp) return IopCleanupFailedIrp(FileObject, NULL);
 
     /* Set the IRP */
     Irp->Tail.Overlay.OriginalFileObject = FileObject;
+    Irp->Tail.Overlay.Thread = PsGetCurrentThread();
     Irp->RequestorMode = ExGetPreviousMode();
     Irp->UserIosb = &IoStatusBlock;
     Irp->UserEvent = (LocalEvent) ? &Event : NULL;
     Irp->Flags = (LocalEvent) ? IRP_SYNCHRONOUS_API : 0;
-    Irp->Tail.Overlay.Thread = PsGetCurrentThread();
+    Irp->Overlay.AsynchronousParameters.UserApcRoutine = NULL;
 
     /* Set Stack Parameters */
     StackPtr = IoGetNextIrpStackLocation(Irp);
-    StackPtr->MajorFunction = MajorFunction;
     StackPtr->FileObject = FileObject;
 
-    /* Set Parameters */
+    /* Check if this is a query or set */
     if (OperationCode == QuerySecurityDescriptor)
     {
-        StackPtr->Parameters.QuerySecurity.SecurityInformation = SecurityInformation;
+        /* Set the major function and parameters */
+        StackPtr->MajorFunction = IRP_MJ_QUERY_SECURITY;
+        StackPtr->Parameters.QuerySecurity.SecurityInformation =
+            SecurityInformation;
         StackPtr->Parameters.QuerySecurity.Length = *BufferLength;
         Irp->UserBuffer = SecurityDescriptor;
     }
     else
     {
-        StackPtr->Parameters.SetSecurity.SecurityInformation = SecurityInformation;
-        StackPtr->Parameters.SetSecurity.SecurityDescriptor = SecurityDescriptor;
-    }
-
-    ObReferenceObject(FileObject);
+        /* Set the major function and parameters for a set */
+        StackPtr->MajorFunction = IRP_MJ_SET_SECURITY;
+        StackPtr->Parameters.SetSecurity.SecurityInformation =
+            SecurityInformation;
+        StackPtr->Parameters.SetSecurity.SecurityDescriptor =
+            SecurityDescriptor;
+    }
+
+    /* Queue the IRP */
+    //IopQueueIrpToThread(Irp);
+
+    /* Update operation counts */
+    IopUpdateOperationCount(IopOtherTransfer);
 
     /* Call the Driver */
     Status = IoCallDriver(FileObject->DeviceObject, Irp);
 
-    if (Status == STATUS_PENDING)
-    {
-        if (LocalEvent)
-        {
+    /* Check if this was async I/O */
+    if (LocalEvent)
+    {
+        /* Check if the IRP is pending completion */
+        if (Status == STATUS_PENDING)
+        {
+            /* Wait on the local event */
             KeWaitForSingleObject(&Event,
                                   Executive,
                                   KernelMode,
-                                  FileObject->Flags & FO_ALERTABLE_IO,
+                                  FALSE,
                                   NULL);
             Status = IoStatusBlock.Status;
         }
-        else
-        {
+    }
+    else
+    {
+        /* Check if the IRP is pending completion */
+        if (Status == STATUS_PENDING)
+        {
+            /* Wait on the file obejct */
             KeWaitForSingleObject(&FileObject->Event,
                                   Executive,
                                   KernelMode,
-                                  FileObject->Flags & FO_ALERTABLE_IO,
+                                  FALSE,
                                   NULL);
             Status = FileObject->FinalStatus;
         }
+
+        /* Release the lock */
+        IopUnlockFileObject(FileObject);
     }
 
     /* This Driver doesn't implement Security, so try to give it a default */
     if (Status == STATUS_INVALID_DEVICE_REQUEST)
     {
+        /* Was this a query? */
         if (OperationCode == QuerySecurityDescriptor)
         {
             /* Set a World Security Descriptor */
@@ -387,6 +451,9 @@
     }
     else if (OperationCode == QuerySecurityDescriptor)
     {
+        /* Callers usually expect the normalized form */
+        if (Status == STATUS_BUFFER_OVERFLOW) Status = STATUS_BUFFER_TOO_SMALL;
+
         /* Return length */
         *BufferLength = IoStatusBlock.Information;
     }

Modified: trunk/reactos/ntoskrnl/io/iomgr/iofunc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/iofunc.c…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/iofunc.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/iofunc.c Wed Jul  5 02:28:23 2006
@@ -34,74 +34,6 @@
 ///
 
 /* PRIVATE FUNCTIONS *********************************************************/
-
-/* DON'T inline this: it's a failure case */
-NTSTATUS
-NTAPI
-IopCleanupFailedIrp(IN PFILE_OBJECT FileObject,
-                    IN PKEVENT EventObject)
-{
-    PAGED_CODE();
-
-    /* Dereference the event */
-    if (EventObject) ObDereferenceObject(EventObject);
-
-    /* If this was a file opened for synch I/O, then unlock it */
-    if (FileObject->Flags & FO_SYNCHRONOUS_IO) IopUnlockFileObject(FileObject);
-
-    /* Now dereference it and return */
-    ObDereferenceObject(FileObject);
-    return STATUS_INSUFFICIENT_RESOURCES;
-}
-
-/* DON'T inline this: it's a failure case */
-VOID
-NTAPI
-IopAbortInterruptedIrp(IN PKEVENT EventObject,
-                       IN PIRP Irp)
-{
-    KIRQL OldIrql;
-    BOOLEAN CancelResult;
-    LARGE_INTEGER Wait;
-    PAGED_CODE();
-
-    /* Raise IRQL to APC */
-    KeRaiseIrql(APC_LEVEL, &OldIrql);
-
-    /* Check if nobody completed it yet */
-    if (!KeReadStateEvent(EventObject))
-    {
-        /* First, cancel it */
-        CancelResult = IoCancelIrp(Irp);
-        KeLowerIrql(OldIrql);
-
-        /* Check if we cancelled it */
-        if (CancelResult)
-        {
-            /* Wait for the IRP to be cancelled */
-            Wait.QuadPart = -100000;
-            while (!KeReadStateEvent(EventObject))
-            {
-                /* Delay indefintely */
-                KeDelayExecutionThread(KernelMode, FALSE, &Wait);
-            }
-        }
-        else
-        {
-            /* No cancellation done, so wait for the I/O system to kill it */
-            KeWaitForSingleObject(EventObject,
-                                  Executive,
-                                  KernelMode,
-                                  FALSE,
-                                  NULL);
-        }
-    }
-    else
-    {
-        /* We got preempted, so give up */
-        KeLowerIrql(OldIrql);
-    }
-}
 
 NTSTATUS
 NTAPI

Modified: trunk/reactos/ntoskrnl/io/iomgr/irp.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/irp.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/irp.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/irp.c Wed Jul  5 02:28:23 2006
@@ -38,6 +38,72 @@
 {
     /* Free the IRP */
     IoFreeIrp(CONTAINING_RECORD(Apc, IRP, Tail.Apc));
+}
+
+NTSTATUS
+NTAPI
+IopCleanupFailedIrp(IN PFILE_OBJECT FileObject,
+                    IN PKEVENT EventObject)
+{
+    PAGED_CODE();
+
+    /* Dereference the event */
+    if (EventObject) ObDereferenceObject(EventObject);
+
+    /* If this was a file opened for synch I/O, then unlock it */
+    if (FileObject->Flags & FO_SYNCHRONOUS_IO) IopUnlockFileObject(FileObject);
+
+    /* Now dereference it and return */
+    ObDereferenceObject(FileObject);
+    return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+VOID
+NTAPI
+IopAbortInterruptedIrp(IN PKEVENT EventObject,
+                       IN PIRP Irp)
+{
+    KIRQL OldIrql;
+    BOOLEAN CancelResult;
+    LARGE_INTEGER Wait;
+    PAGED_CODE();
+
+    /* Raise IRQL to APC */
+    KeRaiseIrql(APC_LEVEL, &OldIrql);
+
+    /* Check if nobody completed it yet */
+    if (!KeReadStateEvent(EventObject))
+    {
+        /* First, cancel it */
+        CancelResult = IoCancelIrp(Irp);
+        KeLowerIrql(OldIrql);
+
+        /* Check if we cancelled it */
+        if (CancelResult)
+        {
+            /* Wait for the IRP to be cancelled */
+            Wait.QuadPart = -100000;
+            while (!KeReadStateEvent(EventObject))
+            {
+                /* Delay indefintely */
+                KeDelayExecutionThread(KernelMode, FALSE, &Wait);
+            }
+        }
+        else
+        {
+            /* No cancellation done, so wait for the I/O system to kill it */
+            KeWaitForSingleObject(EventObject,
+                                  Executive,
+                                  KernelMode,
+                                  FALSE,
+                                  NULL);
+        }
+    }
+    else
+    {
+        /* We got preempted, so give up */
+        KeLowerIrql(OldIrql);
+    }
 }
 
 VOID