Author: hbelusca Date: Wed Dec 5 23:21:41 2012 New Revision: 57806
URL: http://svn.reactos.org/svn/reactos?rev=57806&view=rev Log: [CSRSRV] - Comment on the size of some members of the CSR_WAIT_BLOCK structure. - Initialize the WaitBlock member of CSR_THREAD to a valid value when creating a wait block, and NULLify it when we release a wait block. - ALWAYS USE offsets in CSR_CAPTURE_BUFFER structure, instead of real pointers !! It is needed when their base address change (eg. during a CSR wait, their base address, corresponding to the address of an CSR API message, change) (found when testing CSR waits with the console).
Modified: branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h branches/ros-csrss/subsystems/win32/csrsrv/api.c branches/ros-csrss/subsystems/win32/csrsrv/wait.c
Modified: branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/include/reactos/subsys... ============================================================================== --- branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h [iso-8859-1] (original) +++ branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h [iso-8859-1] Wed Dec 5 23:21:41 2012 @@ -156,13 +156,13 @@
typedef struct _CSR_WAIT_BLOCK { - ULONG Size; + ULONG Size; // Size of the wait block (variable-sized) LIST_ENTRY WaitList; LIST_ENTRY UserWaitList; PVOID WaitContext; PCSR_THREAD WaitThread; CSR_WAIT_FUNCTION WaitFunction; - CSR_API_MESSAGE WaitApiMessage; + CSR_API_MESSAGE WaitApiMessage; // Variable-sized CSR API message } CSR_WAIT_BLOCK, *PCSR_WAIT_BLOCK;
Modified: branches/ros-csrss/subsystems/win32/csrsrv/api.c URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrsr... ============================================================================== --- branches/ros-csrss/subsystems/win32/csrsrv/api.c [iso-8859-1] (original) +++ branches/ros-csrss/subsystems/win32/csrsrv/api.c [iso-8859-1] Wed Dec 5 23:21:41 2012 @@ -504,7 +504,7 @@ if ((ServerDll) && (ServerDll->HardErrorCallback)) { /* Call it */ - ServerDll->HardErrorCallback(NULL /* CsrThread == NULL */, HardErrorMsg); + ServerDll->HardErrorCallback(NULL /* == CsrThread */, HardErrorMsg);
/* If it's handled, get out of here */ if (HardErrorMsg->Response != ResponseNotHandled) break; @@ -524,6 +524,7 @@ else { ReplyMsg = &ReceiveMsg; + ReplyPort = CsrApiPort; } } else if (MessageType == LPC_REQUEST) @@ -1178,20 +1179,21 @@ BufferDistance = (ULONG_PTR)RemoteCaptureBuffer - (ULONG_PTR)LocalCaptureBuffer;
/* - * Convert all the pointer offsets into real pointers, and make - * them point to the remote data buffer instead of the local one. + * All the pointer offsets correspond to pointers which point + * to the remote data buffer instead of the local one. */ for (i = 0 ; i < RemoteCaptureBuffer->PointerCount ; ++i) { if (RemoteCaptureBuffer->PointerOffsetsArray[i] != 0) { + /* Temporarily transform the offset into a pointer */ RemoteCaptureBuffer->PointerOffsetsArray[i] += (ULONG_PTR)ApiMessage;
- /* Validate the bounds of the current pointer */ + /* Validate the bounds of the current pointed pointer */ if ((*(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] >= CsrThread->Process->ClientViewBase) && (*(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] < CsrThread->Process->ClientViewBounds)) { - /* Modify the pointer to take into account its new position */ + /* Modify the pointed pointer to take into account its new position */ *(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] += BufferDistance; } else @@ -1201,19 +1203,22 @@ DbgBreakPoint(); ApiMessage->Status = STATUS_INVALID_PARAMETER; } + + /* Transform back into an offset */ + RemoteCaptureBuffer->PointerOffsetsArray[i] -= (ULONG_PTR)ApiMessage; } }
/* Check if we got success */ if (ApiMessage->Status != STATUS_SUCCESS) { - /* Failure. Free the buffer and return*/ + /* Failure. Free the buffer and return */ RtlFreeHeap(CsrHeap, 0, RemoteCaptureBuffer); return FALSE; } else { - /* Success, save the previous buffer */ + /* Success, save the previous buffer and use the remote capture buffer */ RemoteCaptureBuffer->PreviousCaptureBuffer = LocalCaptureBuffer; ApiMessage->CsrCaptureData = RemoteCaptureBuffer; } @@ -1246,29 +1251,38 @@ SIZE_T BufferDistance; ULONG i;
- /* Get the capture buffers */ + /* Get the remote capture buffer */ RemoteCaptureBuffer = ApiMessage->CsrCaptureData; - LocalCaptureBuffer = RemoteCaptureBuffer->PreviousCaptureBuffer;
/* Do not continue if there is no captured buffer */ if (!RemoteCaptureBuffer) return;
- /* Free the previous one */ + /* If there is one, get the corresponding local capture buffer */ + LocalCaptureBuffer = RemoteCaptureBuffer->PreviousCaptureBuffer; + + /* Free the previous one and use again the local capture buffer */ RemoteCaptureBuffer->PreviousCaptureBuffer = NULL; + ApiMessage->CsrCaptureData = LocalCaptureBuffer;
/* Calculate the difference between our buffer and the client's */ BufferDistance = (ULONG_PTR)RemoteCaptureBuffer - (ULONG_PTR)LocalCaptureBuffer;
/* - * Convert back all the pointers into pointer offsets, and make them - * point to the local data buffer instead of the remote one (revert + * All the pointer offsets correspond to pointers which point + * to the local data buffer instead of the remote one (revert * the logic of CsrCaptureArguments). */ for (i = 0 ; i < RemoteCaptureBuffer->PointerCount ; ++i) { if (RemoteCaptureBuffer->PointerOffsetsArray[i] != 0) { + /* Temporarily transform the offset into a pointer */ + RemoteCaptureBuffer->PointerOffsetsArray[i] += (ULONG_PTR)ApiMessage; + + /* Modify the pointed pointer to take into account its new position */ *(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] -= BufferDistance; + + /* Transform back into an offset */ RemoteCaptureBuffer->PointerOffsetsArray[i] -= (ULONG_PTR)ApiMessage; } } @@ -1313,7 +1327,7 @@ IN ULONG ElementSize) { PCSR_CAPTURE_BUFFER CaptureBuffer = ApiMessage->CsrCaptureData; - // SIZE_T BufferDistance = (ULONG_PTR)Buffer - (ULONG_PTR)ApiMessage; + SIZE_T BufferDistance = (ULONG_PTR)Buffer - (ULONG_PTR)ApiMessage; ULONG i;
/* @@ -1353,10 +1367,10 @@ for (i = 0 ; i < CaptureBuffer->PointerCount ; ++i) { /* - * If the pointer offset is in fact equal to the - * real address of the buffer then it's OK. + * The pointer offset must be equal to the delta between + * the addresses of the buffer and of the API message. */ - if (CaptureBuffer->PointerOffsetsArray[i] == (ULONG_PTR)Buffer /* BufferDistance + (ULONG_PTR)ApiMessage */) + if (CaptureBuffer->PointerOffsetsArray[i] == BufferDistance) { return TRUE; }
Modified: branches/ros-csrss/subsystems/win32/csrsrv/wait.c URL: http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrsr... ============================================================================== --- branches/ros-csrss/subsystems/win32/csrsrv/wait.c [iso-8859-1] (original) +++ branches/ros-csrss/subsystems/win32/csrsrv/wait.c [iso-8859-1] Wed Dec 5 23:21:41 2012 @@ -73,6 +73,7 @@ /* Initialize it */ WaitBlock->Size = Size; WaitBlock->WaitThread = CsrWaitThread; + CsrWaitThread->WaitBlock = WaitBlock; WaitBlock->WaitContext = WaitContext; WaitBlock->WaitFunction = WaitFunction; WaitBlock->UserWaitList.Flink = NULL; @@ -242,6 +243,7 @@ if (CsrWaitThread->Flags & CsrThreadTerminated) { /* Fail the wait */ + CsrWaitThread->WaitBlock = NULL; RtlFreeHeap(CsrHeap, 0, WaitBlock); CsrReleaseWaitLock(); return FALSE;