[ros-diffs] 01/01: [NTOS:IO] Implement IopAcquireFileObjectLock and use it to fix IopLockFileObject

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8fbc4880505a714b73936…

commit 8fbc4880505a714b73936d04f974d0cc44ab069b
Author:     Thomas Faber <thomas.faber(a)reactos.org>
AuthorDate: Tue Oct 2 09:55:07 2018 +0200
Commit:     Pierre Schweitzer <pierre(a)reactos.org>
CommitDate: Tue Oct 2 09:56:55 2018 +0200

    [NTOS:IO] Implement IopAcquireFileObjectLock and use it to fix IopLockFileObject
---
 ntoskrnl/include/internal/io.h   |  9 ++++
 ntoskrnl/include/internal/io_x.h | 30 +++++++++----
 ntoskrnl/io/iomgr/file.c         | 57 ++++++++++++++++++++++--
 ntoskrnl/io/iomgr/iofunc.c       | 96 ++++++++++++++++++++++++++++++++++------
 4 files changed, 167 insertions(+), 25 deletions(-)

diff --git a/ntoskrnl/include/internal/io.h b/ntoskrnl/include/internal/io.h
index 83fe7ce3c7..97df17f2ea 100644
--- a/ntoskrnl/include/internal/io.h
+++ b/ntoskrnl/include/internal/io.h
@@ -1238,6 +1238,15 @@ IopCloseFile(
     IN ULONG SystemHandleCount
 );
 
+NTSTATUS
+NTAPI
+IopAcquireFileObjectLock(
+    _In_ PFILE_OBJECT FileObject,
+    _In_ KPROCESSOR_MODE AccessMode,
+    _In_ BOOLEAN Alertable,
+    _Out_ PBOOLEAN LockFailed
+);
+
 PVOID
 NTAPI
 IoGetFileObjectFilterContext(
diff --git a/ntoskrnl/include/internal/io_x.h b/ntoskrnl/include/internal/io_x.h
index 7e0d028d94..7cf42e9dc0 100644
--- a/ntoskrnl/include/internal/io_x.h
+++ b/ntoskrnl/include/internal/io_x.h
@@ -8,16 +8,26 @@
 
 static
 __inline
-VOID
-IopLockFileObject(IN PFILE_OBJECT FileObject)
+NTSTATUS
+IopLockFileObject(
+    _In_ PFILE_OBJECT FileObject,
+    _In_ KPROCESSOR_MODE WaitMode)
 {
+    BOOLEAN LockFailed;
+
     /* Lock the FO and check for contention */
-    InterlockedIncrement((PLONG)&FileObject->Waiters);
-    while (InterlockedCompareExchange((PLONG)&FileObject->Busy, TRUE, FALSE) !=
FALSE)
+    if (InterlockedExchange((PLONG)&FileObject->Busy, TRUE) == FALSE)
+    {
+        ObReferenceObject(FileObject);
+        return STATUS_SUCCESS;
+    }
+    else
     {
-        /* FIXME - pause for a little while? */
+        return IopAcquireFileObjectLock(FileObject,
+                                        WaitMode,
+                                        BooleanFlagOn(FileObject->Flags,
FO_ALERTABLE_IO),
+                                        &LockFailed);
     }
-    InterlockedDecrement((PLONG)&FileObject->Waiters);
 }
 
 static
@@ -26,8 +36,12 @@ VOID
 IopUnlockFileObject(IN PFILE_OBJECT FileObject)
 {
     /* Unlock the FO and wake any waiters up */
-    InterlockedExchange((PLONG)&FileObject->Busy, FALSE);
-    if (FileObject->Waiters) KeSetEvent(&FileObject->Lock, 0, FALSE);
+    NT_VERIFY(InterlockedExchange((PLONG)&FileObject->Busy, FALSE) == TRUE);
+    if (FileObject->Waiters)
+    {
+        KeSetEvent(&FileObject->Lock, IO_NO_INCREMENT, FALSE);
+    }
+    ObDereferenceObject(FileObject);
 }
 
 FORCEINLINE
diff --git a/ntoskrnl/io/iomgr/file.c b/ntoskrnl/io/iomgr/file.c
index 1768bc29ca..6133c9e45d 100644
--- a/ntoskrnl/io/iomgr/file.c
+++ b/ntoskrnl/io/iomgr/file.c
@@ -1729,7 +1729,12 @@ IopGetSetSecurityObject(IN PVOID ObjectBody,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock the file object */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, ExGetPreviousMode());
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
     }
     else
     {
@@ -2026,7 +2031,7 @@ IopCloseFile(IN PEPROCESS Process OPTIONAL,
         /* Check if this is a sync FO and lock it */
         if (BooleanFlagOn(FileObject->Flags, FO_SYNCHRONOUS_IO))
         {
-            IopLockFileObject(FileObject);
+            (VOID)IopLockFileObject(FileObject, KernelMode);
         }
 
         /* Go the FastIO path if possible, otherwise fall back to IRP */
@@ -2100,7 +2105,7 @@ IopCloseFile(IN PEPROCESS Process OPTIONAL,
     if (Process != NULL &&
         BooleanFlagOn(FileObject->Flags, FO_SYNCHRONOUS_IO))
     {
-        IopLockFileObject(FileObject);
+        (VOID)IopLockFileObject(FileObject, KernelMode);
     }
 
     /* Clear and set up Events */
@@ -2259,6 +2264,52 @@ IopQueryAttributesFile(IN POBJECT_ATTRIBUTES ObjectAttributes,
     return Status;
 }
 
+NTSTATUS
+NTAPI
+IopAcquireFileObjectLock(
+    _In_ PFILE_OBJECT FileObject,
+    _In_ KPROCESSOR_MODE WaitMode,
+    _In_ BOOLEAN Alertable,
+    _Out_ PBOOLEAN LockFailed)
+{
+    NTSTATUS Status;
+
+    PAGED_CODE();
+
+    InterlockedIncrement((PLONG)&FileObject->Waiters);
+
+    Status = STATUS_SUCCESS;
+    do
+    {
+        if (!InterlockedExchange((PLONG)&FileObject->Busy, TRUE))
+        {
+            break;
+        }
+        Status = KeWaitForSingleObject(&FileObject->Lock,
+                                       Executive,
+                                       WaitMode,
+                                       Alertable,
+                                       NULL);
+    } while (Status == STATUS_SUCCESS);
+
+    InterlockedDecrement((PLONG)&FileObject->Waiters);
+    if (Status == STATUS_SUCCESS)
+    {
+        ObReferenceObject(FileObject);
+        *LockFailed = FALSE;
+    }
+    else
+    {
+        if (!FileObject->Busy && FileObject->Waiters)
+        {
+            KeSetEvent(&FileObject->Lock, IO_NO_INCREMENT, FALSE);
+        }
+        *LockFailed = TRUE;
+    }
+
+    return Status;
+}
+
 PVOID
 NTAPI
 IoGetFileObjectFilterContext(IN PFILE_OBJECT FileObject)
diff --git a/ntoskrnl/io/iomgr/iofunc.c b/ntoskrnl/io/iomgr/iofunc.c
index d119f851e1..2ee72479cf 100644
--- a/ntoskrnl/io/iomgr/iofunc.c
+++ b/ntoskrnl/io/iomgr/iofunc.c
@@ -334,7 +334,13 @@ IopDeviceFsIoControl(IN HANDLE DeviceHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (EventObject) ObDereferenceObject(EventObject);
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
 
         /* Remember to unlock later */
         LockedForSynch = TRUE;
@@ -666,7 +672,7 @@ IopQueryDeviceInformation(IN PFILE_OBJECT FileObject,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        (void)IopLockFileObject(FileObject, KernelMode);
 
         /* Use File Object event */
         KeClearEvent(&FileObject->Event);
@@ -1221,7 +1227,7 @@ IoSetInformation(IN PFILE_OBJECT FileObject,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        (void)IopLockFileObject(FileObject, KernelMode);
 
         /* Use File Object event */
         KeClearEvent(&FileObject->Event);
@@ -1431,7 +1437,12 @@ NtFlushBuffersFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
     }
     else
     {
@@ -1579,7 +1590,13 @@ NtNotifyChangeDirectoryFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (Event) ObDereferenceObject(Event);
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
         LockedForSync = TRUE;
     }
 
@@ -1779,7 +1796,13 @@ NtLockFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (Event) ObDereferenceObject(Event);
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
         LockedForSync = TRUE;
     }
 
@@ -1972,7 +1995,14 @@ NtQueryDirectoryFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (Event) ObDereferenceObject(Event);
+            ObDereferenceObject(FileObject);
+            if (AuxBuffer) ExFreePoolWithTag(AuxBuffer, TAG_SYSB);
+            return Status;
+        }
 
         /* Remember to unlock later */
         LockedForSynch = TRUE;
@@ -2207,7 +2237,12 @@ NtQueryInformationFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
 
         /* Check if the caller just wants the position */
         if (FileInformationClass == FilePositionInformation)
@@ -2619,7 +2654,13 @@ NtReadFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock the file object */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (EventObject) ObDereferenceObject(EventObject);
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
 
         /* Check if we don't have a byte offset available */
         if (!(ByteOffset) ||
@@ -2961,7 +3002,12 @@ NtSetInformationFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
 
         /* Check if the caller just wants the position */
         if (FileInformationClass == FilePositionInformation)
@@ -3411,7 +3457,12 @@ NtUnlockFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
     }
     else
     {
@@ -3616,7 +3667,13 @@ NtWriteFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock the file object */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            if (EventObject) ObDereferenceObject(EventObject);
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
 
         /* Check if we don't have a byte offset available */
         if (!(ByteOffset) ||
@@ -3897,7 +3954,12 @@ NtQueryVolumeInformationFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            return Status;
+        }
     }
     else
     {
@@ -4068,7 +4130,13 @@ NtSetVolumeInformationFile(IN HANDLE FileHandle,
     if (FileObject->Flags & FO_SYNCHRONOUS_IO)
     {
         /* Lock it */
-        IopLockFileObject(FileObject);
+        Status = IopLockFileObject(FileObject, PreviousMode);
+        if (Status != STATUS_SUCCESS)
+        {
+            ObDereferenceObject(FileObject);
+            if (TargetDeviceObject) ObDereferenceObject(TargetDeviceObject);
+            return Status;
+        }
     }
     else
     {