[VIDEOPRT] Improve IntAttachToCSRSS/IntDetachFromCSRSS() prototypes (reactos#8449)

- Make `IntAttachToCSRSS()` return a BOOLEAN to distinguish calls made
  prior to `CsrProcess` being initialized.

- Adjust the callers of `IntAttachToCSRSS()` and make them returning a
  proper error value if attaching failed (if `CsrProcess == NULL`).

- Make `IntDetachFromCSRSS()` just take a `PKPROCESS` parameter instead a
  pointer to `PKPROCESS` -- the function won't need to modify its value.
  Adjust its callers to reflect the change.

Author: HBelusca

win32ss/drivers/videoprt/dispatch.c

@@ -43,7 +43,7 @@ VideoPortWin32kCallout(
     if (!Win32kCallout)
         return;
 
-    /* Perform the call in the context of CSRSS */
+    /* Perform the call in the CSRSS context */
     if (!CsrProcess)
         return;
 
@@ -378,10 +378,8 @@ IntVideoPortDispatchOpen(
 
     if (!CsrProcess)
     {
-        /*
-         * We know the first open call will be from the CSRSS process
-         * to let us know its handle.
-         */
+        /* We know the first open call is from the CSRSS process.
+         * Get a reference to it for Int10 support. */
         INFO_(VIDEOPRT, "Referencing CSRSS\n");
         CsrProcess = (PKPROCESS)PsGetCurrentProcess();
         ObReferenceObject(CsrProcess);

win32ss/drivers/videoprt/int10.c

@@ -33,7 +33,7 @@
 #define IsLowV86Mem(_Seg, _Off) ((((_Seg) << 4) + (_Off)) < (0xa0000))
 
 /* Those two functions below are there so that CSRSS can't access low mem.
- * Expecially, MAKE IT CRASH ON NULL ACCESS */
+ * Especially, MAKE IT CRASH ON NULL ACCESS */
 static
 VOID
 ProtectLowV86Mem(VOID)
@@ -44,7 +44,7 @@ ProtectLowV86Mem(VOID)
     NTSTATUS Status;
     SIZE_T ViewSize = 0xa0000 - PAGE_SIZE;
 
-    /* We should only do that for CSRSS. */
+    /* We should only do that for CSRSS */
     ASSERT(PsGetCurrentProcess() == (PEPROCESS)CsrProcess);
 
     /* Commit (again) the pages, but with PAGE_NOACCESS protection */
@@ -96,6 +96,9 @@ IntInitializeVideoAddressSpace(VOID)
     CHAR IVTAndBda[1024 + 256];
 #endif // _M_IX86
 
+    /* We should only do that for CSRSS */
+    ASSERT(PsGetCurrentProcess() == (PEPROCESS)CsrProcess);
+
     /* Free the 1MB pre-reserved region. In reality, ReactOS should simply support us mapping the view into the reserved area, but it doesn't. */
     BaseAddress = 0;
     ViewSize = 1024 * 1024;
@@ -216,13 +219,15 @@ IntInt10AllocateBuffer(
     NTSTATUS Status;
 #ifdef _M_IX86
     PVOID MemoryAddress;
-    PKPROCESS CallingProcess = (PKPROCESS)PsGetCurrentProcess();
+    PKPROCESS CallingProcess;
     KAPC_STATE ApcState;
     SIZE_T Size;
 
     TRACE_(VIDEOPRT, "IntInt10AllocateBuffer\n");
 
-    IntAttachToCSRSS(&CallingProcess, &ApcState);
+    /* Perform the call in the CSRSS context */
+    if (!IntAttachToCSRSS(&CallingProcess, &ApcState))
+        return ERROR_INVALID_PARAMETER;
 
     Size = *Length;
     MemoryAddress = (PVOID)0x20000;
@@ -236,7 +241,7 @@ IntInt10AllocateBuffer(
     if (!NT_SUCCESS(Status))
     {
         WARN_(VIDEOPRT, "- ZwAllocateVirtualMemory failed\n");
-        IntDetachFromCSRSS(&CallingProcess, &ApcState);
+        IntDetachFromCSRSS(CallingProcess, &ApcState);
         return ERROR_NOT_ENOUGH_MEMORY;
     }
 
@@ -247,20 +252,20 @@ IntInt10AllocateBuffer(
                             &Size,
                             MEM_RELEASE);
         WARN_(VIDEOPRT, "- Unacceptable memory allocated\n");
-        IntDetachFromCSRSS(&CallingProcess, &ApcState);
+        IntDetachFromCSRSS(CallingProcess, &ApcState);
         return ERROR_NOT_ENOUGH_MEMORY;
     }
 
+    IntDetachFromCSRSS(CallingProcess, &ApcState);
+
     *Length = (ULONG)Size;
     *Seg = (USHORT)((ULONG_PTR)MemoryAddress >> 4);
     *Off = (USHORT)((ULONG_PTR)MemoryAddress & 0xF);
 
-    INFO_(VIDEOPRT, "- Segment: %x\n", (ULONG_PTR)MemoryAddress >> 4);
-    INFO_(VIDEOPRT, "- Offset: %x\n", (ULONG_PTR)MemoryAddress & 0xF);
+    INFO_(VIDEOPRT, "- Segment: %x\n", *Seg);
+    INFO_(VIDEOPRT, "- Offset: %x\n", *Off);
     INFO_(VIDEOPRT, "- Length: %x\n", *Length);
 
-    IntDetachFromCSRSS(&CallingProcess, &ApcState);
-
     return NO_ERROR;
 #else
     Status = x86BiosAllocateBuffer(Length, Seg, Off);
@@ -278,21 +283,24 @@ IntInt10FreeBuffer(
     NTSTATUS Status;
 #ifdef _M_IX86
     PVOID MemoryAddress = (PVOID)((ULONG_PTR)(Seg << 4) | Off);
-    PKPROCESS CallingProcess = (PKPROCESS)PsGetCurrentProcess();
+    PKPROCESS CallingProcess;
     KAPC_STATE ApcState;
     SIZE_T Size = 0;
 
     TRACE_(VIDEOPRT, "IntInt10FreeBuffer\n");
     INFO_(VIDEOPRT, "- Segment: %x\n", Seg);
     INFO_(VIDEOPRT, "- Offset: %x\n", Off);
 
-    IntAttachToCSRSS(&CallingProcess, &ApcState);
+    /* Perform the call in the CSRSS context */
+    if (!IntAttachToCSRSS(&CallingProcess, &ApcState))
+        return ERROR_INVALID_PARAMETER;
+
     Status = ZwFreeVirtualMemory(NtCurrentProcess(),
                                  &MemoryAddress,
                                  &Size,
                                  MEM_RELEASE);
 
-    IntDetachFromCSRSS(&CallingProcess, &ApcState);
+    IntDetachFromCSRSS(CallingProcess, &ApcState);
 
     return Status;
 #else
@@ -311,7 +319,7 @@ IntInt10ReadMemory(
     IN ULONG Length)
 {
 #ifdef _M_IX86
-    PKPROCESS CallingProcess = (PKPROCESS)PsGetCurrentProcess();
+    PKPROCESS CallingProcess;
     KAPC_STATE ApcState;
 
     TRACE_(VIDEOPRT, "IntInt10ReadMemory\n");
@@ -320,15 +328,17 @@ IntInt10ReadMemory(
     INFO_(VIDEOPRT, "- Buffer: %x\n", Buffer);
     INFO_(VIDEOPRT, "- Length: %x\n", Length);
 
-    IntAttachToCSRSS(&CallingProcess, &ApcState);
+    /* Perform the call in the CSRSS context */
+    if (!IntAttachToCSRSS(&CallingProcess, &ApcState))
+        return ERROR_INVALID_PARAMETER;
 
     if (IsLowV86Mem(Seg, Off))
         UnprotectLowV86Mem();
     RtlCopyMemory(Buffer, (PVOID)((ULONG_PTR)(Seg << 4) | Off), Length);
     if (IsLowV86Mem(Seg, Off))
         ProtectLowV86Mem();
 
-    IntDetachFromCSRSS(&CallingProcess, &ApcState);
+    IntDetachFromCSRSS(CallingProcess, &ApcState);
 
     return NO_ERROR;
 #else
@@ -349,7 +359,7 @@ IntInt10WriteMemory(
     IN ULONG Length)
 {
 #ifdef _M_IX86
-    PKPROCESS CallingProcess = (PKPROCESS)PsGetCurrentProcess();
+    PKPROCESS CallingProcess;
     KAPC_STATE ApcState;
 
     TRACE_(VIDEOPRT, "IntInt10WriteMemory\n");
@@ -358,13 +368,17 @@ IntInt10WriteMemory(
     INFO_(VIDEOPRT, "- Buffer: %x\n", Buffer);
     INFO_(VIDEOPRT, "- Length: %x\n", Length);
 
-    IntAttachToCSRSS(&CallingProcess, &ApcState);
+    /* Perform the call in the CSRSS context */
+    if (!IntAttachToCSRSS(&CallingProcess, &ApcState))
+        return ERROR_INVALID_PARAMETER;
+
     if (IsLowV86Mem(Seg, Off))
         UnprotectLowV86Mem();
     RtlCopyMemory((PVOID)((ULONG_PTR)(Seg << 4) | Off), Buffer, Length);
     if (IsLowV86Mem(Seg, Off))
         ProtectLowV86Mem();
-    IntDetachFromCSRSS(&CallingProcess, &ApcState);
+
+    IntDetachFromCSRSS(CallingProcess, &ApcState);
 
     return NO_ERROR;
 #else
@@ -387,16 +401,11 @@ IntInt10CallBios(
     CONTEXT BiosContext;
 #endif
     NTSTATUS Status;
-    PKPROCESS CallingProcess = (PKPROCESS)PsGetCurrentProcess();
+    PKPROCESS CallingProcess;
     KAPC_STATE ApcState;
 
-    /* Attach to CSRSS */
-    IntAttachToCSRSS(&CallingProcess, &ApcState);
-
-    /* Clear the context */
+    /* Clear the context and fill out the BIOS arguments */
     RtlZeroMemory(&BiosContext, sizeof(BiosContext));
-
-    /* Fill out the bios arguments */
     BiosContext.Eax = BiosArguments->Eax;
     BiosContext.Ebx = BiosArguments->Ebx;
     BiosContext.Ecx = BiosArguments->Ecx;
@@ -407,6 +416,10 @@ IntInt10CallBios(
     BiosContext.SegDs = BiosArguments->SegDs;
     BiosContext.SegEs = BiosArguments->SegEs;
 
+    /* Perform the call in the CSRSS context */
+    if (!IntAttachToCSRSS(&CallingProcess, &ApcState))
+        return ERROR_INVALID_PARAMETER;
+
     /* Do the ROM BIOS call */
     (void)KeWaitForMutexObject(&VideoPortInt10Mutex,
                                Executive,
@@ -425,6 +438,8 @@ IntInt10CallBios(
 
     KeReleaseMutex(&VideoPortInt10Mutex, FALSE);
 
+    IntDetachFromCSRSS(CallingProcess, &ApcState);
+
     /* Return the arguments */
     BiosArguments->Eax = BiosContext.Eax;
     BiosArguments->Ebx = BiosContext.Ebx;
@@ -436,15 +451,7 @@ IntInt10CallBios(
     BiosArguments->SegDs = (USHORT)BiosContext.SegDs;
     BiosArguments->SegEs = (USHORT)BiosContext.SegEs;
 
-    /* Detach and return status */
-    IntDetachFromCSRSS(&CallingProcess, &ApcState);
-
-    if (NT_SUCCESS(Status))
-    {
-        return NO_ERROR;
-    }
-
-    return ERROR_INVALID_PARAMETER;
+    return NT_SUCCESS(Status) ? NO_ERROR : ERROR_INVALID_PARAMETER;
 }
 
 /* PUBLIC FUNCTIONS ***********************************************************/

win32ss/drivers/videoprt/videoprt.c

@@ -570,29 +570,56 @@ IntVideoPortFindAdapter(
     return Status;
 }
 
-VOID
+/**
+ * @brief
+ * Attach the current thread to the CSRSS process. The caller must detach from
+ * the process by invoking IntDetachFromCSRSS() after operating in its context.
+ *
+ * @param[out]  CallingProcess
+ * Pointer to a PKPROCESS variable that receives the current process.
+ *
+ * @param[out]  ApcState
+ * Pointer to a caller-provided KAPC_STATE structure that will be initialized.
+ *
+ * @return
+ * TRUE if attachment succeeded (the CSRSS process exists); FALSE if not.
+ **/
+BOOLEAN
 FASTCALL
 IntAttachToCSRSS(
-    PKPROCESS *CallingProcess,
-    PKAPC_STATE ApcState)
+    _Outptr_ PKPROCESS* CallingProcess,
+    _Out_ PKAPC_STATE ApcState)
 {
+    if (!CsrProcess)
+        return FALSE;
+
     *CallingProcess = (PKPROCESS)PsGetCurrentProcess();
     if (*CallingProcess != CsrProcess)
-    {
         KeStackAttachProcess(CsrProcess, ApcState);
-    }
+    return TRUE;
 }
 
+/**
+ * @brief
+ * Detach the current thread from the CSRSS process. This routine is
+ * to be invoked after a previous successful IntAttachToCSRSS() call.
+ *
+ * @param[in]   CallingProcess
+ * The calling process that previously invoked IntAttachToCSRSS().
+ *
+ * @param[in]   ApcState
+ * Pointer to the KAPC_STATE structure that was initialized by a
+ * previous IntAttachToCSRSS() call.
+ **/
 VOID
 FASTCALL
 IntDetachFromCSRSS(
-    PKPROCESS *CallingProcess,
-    PKAPC_STATE ApcState)
+    _In_ PKPROCESS CallingProcess,
+    _In_ PKAPC_STATE ApcState)
 {
-    if (*CallingProcess != CsrProcess)
-    {
+    ASSERT(CsrProcess);
+    if (CallingProcess != CsrProcess)
         KeUnstackDetachProcess(ApcState);
-    }
 }
 
 VOID
@@ -1154,7 +1181,7 @@ VideoPortGetRomImage(
     TRACE_(VIDEOPRT, "VideoPortGetRomImage(HwDeviceExtension 0x%X Length 0x%X)\n",
            HwDeviceExtension, Length);
 
-    /* If the length is zero then free the existing buffer. */
+    /* If the length is zero then free the existing buffer */
     if (Length == 0)
     {
         if (RomImageBuffer != NULL)
@@ -1168,28 +1195,31 @@ VideoPortGetRomImage(
     {
         /*
          * The DDK says we shouldn't use the legacy C0000 method but get the
-         * rom base address from the corresponding pci or acpi register but
+         * ROM base address from the corresponding PCI or ACPI register but
          * lets ignore that and use C0000 anyway. We have already mapped the
-         * bios area into memory so we'll copy from there.
+         * BIOS area into memory so we'll copy from there.
          */
 
-        /* Copy the bios. */
+        /* Copy the BIOS */
         Length = min(Length, 0x10000);
         if (RomImageBuffer != NULL)
-        {
             ExFreePool(RomImageBuffer);
-        }
 
         RomImageBuffer = ExAllocatePool(PagedPool, Length);
         if (RomImageBuffer == NULL)
-        {
             return NULL;
-        }
-
-        IntAttachToCSRSS(&CallingProcess, &ApcState);
-        RtlCopyMemory(RomImageBuffer, (PUCHAR)0xC0000, Length);
-        IntDetachFromCSRSS(&CallingProcess, &ApcState);
 
+        /* Perform the copy in the CSRSS context */
+        if (IntAttachToCSRSS(&CallingProcess, &ApcState))
+        {
+            RtlCopyMemory(RomImageBuffer, (PUCHAR)0xC0000, Length);
+            IntDetachFromCSRSS(CallingProcess, &ApcState);
+        }
+        else
+        {
+            ExFreePool(RomImageBuffer);
+            RomImageBuffer = NULL;
+        }
         return RomImageBuffer;
     }
 }

win32ss/drivers/videoprt/videoprt.h

@@ -258,11 +258,17 @@ extern KMUTEX VideoPortInt10Mutex;
 extern KSPIN_LOCK HwResetAdaptersLock;
 extern LIST_ENTRY HwResetAdaptersList;
 
-VOID FASTCALL
-IntAttachToCSRSS(PKPROCESS *CallingProcess, PKAPC_STATE ApcState);
+BOOLEAN
+FASTCALL
+IntAttachToCSRSS(
+    _Outptr_ PKPROCESS* CallingProcess,
+    _Out_ PKAPC_STATE ApcState);
 
-VOID FASTCALL
-IntDetachFromCSRSS(PKPROCESS *CallingProcess, PKAPC_STATE ApcState);
+VOID
+FASTCALL
+IntDetachFromCSRSS(
+    _In_ PKPROCESS CallingProcess,
+    _In_ PKAPC_STATE ApcState);
 
 NTSTATUS NTAPI
 IntVideoPortCreateAdapterDeviceObject(