[ros-diffs] [reactos] 02/03: [NTOS/CC] Fix broken usage of _SEH2_FINALLY

[Timo Kreuzer]

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

commit cb74d9e24ed16d3d52e1a4cff3ff3fde6bcd9bd1
Author:     Timo Kreuzer <timo.kreuzer(a)reactos.org>
AuthorDate: Tue Dec 27 17:43:10 2022 +0200
Commit:     Timo Kreuzer <timo.kreuzer(a)reactos.org>
CommitDate: Fri Apr 14 11:56:08 2023 +0300

    [NTOS/CC] Fix broken usage of _SEH2_FINALLY
    
    Finally handlers are - unlike except blocks - not part of the function they are in,
but separate functions, which are called during unwind. PSEH implements them on GCC using
nested functions. While "return" from a finally handler is allowed with native
SEH, it's handled by the compiler through an extra unwinding operation using
_local_unwind, WHICH IS NOT SUPPORTED BY PSEH! With PSEH, returning from a finally handler
does not return from the function, instead it will only return from  [...]
    Also use _SEH_VOLATILE to make sure the variable assignment is not optimized away by
the compiler and add zero out the result parameters on error.
---
 ntoskrnl/cc/pin.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index 3d80befb660..dcd1d5483a5 100644
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -221,7 +221,7 @@ CcpPinData(
     KIRQL OldIrql;
     ULONG VacbOffset;
     NTSTATUS Status;
-    BOOLEAN Result;
+    _SEH2_VOLATILE BOOLEAN Result;
 
     VacbOffset = (ULONG)(FileOffset->QuadPart % VACB_MAPPING_GRANULARITY);
 
@@ -302,15 +302,19 @@ CcpPinData(
             CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu
Flags=0x%lx -> FALSE\n",
                             SharedCacheMap->FileObject, FileOffset, Length, Flags);
             CcUnpinData(&NewBcb->PFCB);
-            return FALSE;
+            *Bcb = NULL;
+            *Buffer = NULL;
         }
     }
     _SEH2_END;
 
-    *Bcb = &NewBcb->PFCB;
-    *Buffer = (PVOID)((ULONG_PTR)NewBcb->Vacb->BaseAddress + VacbOffset);
+    if (Result)
+    {
+        *Bcb = &NewBcb->PFCB;
+        *Buffer = (PVOID)((ULONG_PTR)NewBcb->Vacb->BaseAddress + VacbOffset);
+    }
 
-    return TRUE;
+    return Result;
 }
 
 /*
@@ -332,7 +336,7 @@ CcMapData (
     PROS_SHARED_CACHE_MAP SharedCacheMap;
     ULONG VacbOffset;
     NTSTATUS Status;
-    BOOLEAN Result;
+    _SEH2_VOLATILE BOOLEAN Result;
 
     CCTRACE(CC_API_DEBUG, "CcMapData(FileObject 0x%p, FileOffset 0x%I64x, Length
%lu, Flags 0x%lx,"
            " pBcb 0x%p, pBuffer 0x%p)\n", FileObject, FileOffset->QuadPart,
@@ -406,13 +410,17 @@ CcMapData (
         if (!Result)
         {
             CcpDereferenceBcb(SharedCacheMap, iBcb);
-            return FALSE;
+            *pBcb = NULL;
+            *pBuffer = NULL;
         }
     }
     _SEH2_END;
 
-    *pBcb = &iBcb->PFCB;
-    *pBuffer = (PVOID)((ULONG_PTR)iBcb->Vacb->BaseAddress + VacbOffset);
+    if (Result)
+    {
+        *pBcb = &iBcb->PFCB;
+        *pBuffer = (PVOID)((ULONG_PTR)iBcb->Vacb->BaseAddress + VacbOffset);
+    }
 
     CCTRACE(CC_API_DEBUG, "FileObject=%p FileOffset=%p Length=%lu Flags=0x%lx ->
TRUE Bcb=%p, Buffer %p\n",
         FileObject, FileOffset, Length, Flags, *pBcb, *pBuffer);