[ros-diffs] [mjmartin] 38699: - Last implementation was failing to charge the QuotaAvailable for the message length proceeding the message in the buffer, causing overwriting of the pool. See bug 4018 for more info.
mjmartin at svn.reactos.org
mjmartin at svn.reactos.org
Sun Jan 11 15:28:46 CET 2009
Author: mjmartin
Date: Sun Jan 11 08:28:45 2009
New Revision: 38699
URL: http://svn.reactos.org/svn/reactos?rev=38699&view=rev
Log:
- Last implementation was failing to charge the QuotaAvailable for the message length proceeding the message in the buffer, causing overwriting of the pool. See bug 4018 for more info.
Modified:
trunk/reactos/drivers/filesystems/npfs/rw.c
Modified: trunk/reactos/drivers/filesystems/npfs/rw.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/rw.c?rev=38699&r1=38698&r2=38699&view=diff
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/rw.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/rw.c [iso-8859-1] Sun Jan 11 08:28:45 2009
@@ -438,6 +438,7 @@
break;
}
ExReleaseFastMutex(&Ccb->DataListLock);
+
if (IoIsOperationSynchronous(Irp))
{
/* Wait for ReadEvent to become signaled */
@@ -448,8 +449,7 @@
Irp->RequestorMode,
FALSE,
NULL);
- DPRINT("Finished waiting (%wZ)! Status: %x\n", &Ccb->Fcb->PipeName, Status);
- ExAcquireFastMutex(&Ccb->DataListLock);
+ DPRINT("Finished waiting (%wZ)! Status: %x\n", &Ccb->Fcb->PipeName, Status);
if ((Status == STATUS_USER_APC) || (Status == STATUS_KERNEL_APC))
{
@@ -460,6 +460,7 @@
{
ASSERT(FALSE);
}
+ ExAcquireFastMutex(&Ccb->DataListLock);
}
else
{
@@ -526,26 +527,25 @@
DPRINT("Message mode\n");
/* For Message mode, the Message length will be stored in the buffer preceeding the Message. */
-
if (Ccb->ReadDataAvailable)
{
ULONG NextMessageLength=0;
//HexDump(Ccb->Data, (ULONG)Ccb->WritePtr - (ULONG)Ccb->Data);
-
/*First get the size of the message */
memcpy(&NextMessageLength, Ccb->Data, sizeof(NextMessageLength));
-
- if (NextMessageLength == 0)
+ if ((NextMessageLength == 0) || (NextMessageLength > Ccb->ReadDataAvailable))
{
DPRINT1("This should never happen! Possible memory corruption.\n");
#ifndef NDEBUG
HexDump(Ccb->Data, (ULONG)Ccb->WritePtr - (ULONG)Ccb->Data);
#endif
- break;
+ ASSERT(FALSE);
}
/* Use the smaller value */
+
CopyLength = min(NextMessageLength, Length);
+
/* retrieve the message from the buffer */
memcpy(Buffer, (PVOID)((ULONG)Ccb->Data + sizeof(NextMessageLength)), CopyLength);
@@ -558,6 +558,7 @@
/* Calculate the remaining message new size */
ULONG NewMessageSize = NextMessageLength-CopyLength;
+
/* Write a new Message size to buffer for the part of the message still there */
memcpy(Ccb->Data, &NewMessageSize, sizeof(NewMessageSize));
@@ -568,10 +569,15 @@
/* Update the write pointer */
Ccb->WritePtr = (PVOID)((ULONG)Ccb->WritePtr - CopyLength);
+
+ /* Add CopyLength only to WriteQuotaAvailable as the
+ Message Size was replaced in the buffer */
+ Ccb->WriteQuotaAvailable += CopyLength;
}
else
{
/* Client wanted the entire message */
+
/* Move the memory starting from the next Message just retrieved */
memmove(Ccb->Data,
(PVOID)((ULONG_PTR) Ccb->Data + NextMessageLength + sizeof(NextMessageLength)),
@@ -579,14 +585,24 @@
/* Update the write pointer */
Ccb->WritePtr = (PVOID)((ULONG)Ccb->WritePtr - NextMessageLength);
+
+ /* Add both the message length and the header to the WriteQuotaAvailable
+ as they both were removed */
+ Ccb->WriteQuotaAvailable += (CopyLength + sizeof(NextMessageLength));
}
}
else
{
/* This was the last Message, so just zero this messages for safety sake */
memset(Ccb->Data,0,NextMessageLength + sizeof(NextMessageLength));
- /* reset the write pointer */
+
+ /* reset the write pointer to beginning of buffer */
Ccb->WritePtr = Ccb->Data;
+
+ /* Add both the message length and the header to the WriteQuotaAvailable
+ as they both were removed */
+ Ccb->WriteQuotaAvailable += (CopyLength + sizeof(ULONG));
+
KeResetEvent(&Ccb->ReadEvent);
if (Ccb->PipeState == FILE_PIPE_CONNECTED_STATE)
{
@@ -599,8 +615,10 @@
#endif
Information += CopyLength;
- Ccb->WriteQuotaAvailable +=CopyLength;
+
Ccb->ReadDataAvailable -= CopyLength;
+
+ if ((ULONG)Ccb->WriteQuotaAvailable > (ULONG)Ccb->MaxDataLength) ASSERT(FALSE);
}
if (Information > 0)
@@ -700,7 +718,7 @@
if (Irp->MdlAddress == NULL)
{
- DPRINT("Irp->MdlAddress == NULL\n");
+ DPRINT1("Irp->MdlAddress == NULL\n");
Status = STATUS_UNSUCCESSFUL;
Length = 0;
goto done;
@@ -708,7 +726,7 @@
if (ReaderCcb == NULL)
{
- DPRINT("Pipe is NOT connected!\n");
+ DPRINT1("Pipe is NOT connected!\n");
if (Ccb->PipeState == FILE_PIPE_LISTENING_STATE)
Status = STATUS_PIPE_LISTENING;
else if (Ccb->PipeState == FILE_PIPE_DISCONNECTED_STATE)
@@ -721,7 +739,7 @@
if (ReaderCcb->Data == NULL)
{
- DPRINT("Pipe is NOT writable!\n");
+ DPRINT1("Pipe is NOT writable!\n");
Status = STATUS_UNSUCCESSFUL;
Length = 0;
goto done;
@@ -738,7 +756,7 @@
while(1)
{
- if (ReaderCcb->WriteQuotaAvailable == 0)
+ if ((ReaderCcb->WriteQuotaAvailable == 0))
{
KeSetEvent(&ReaderCcb->ReadEvent, IO_NO_INCREMENT, FALSE);
if (Ccb->PipeState != FILE_PIPE_CONNECTED_STATE)
@@ -749,14 +767,14 @@
}
ExReleaseFastMutex(&ReaderCcb->DataListLock);
- DPRINT("Waiting for buffer space (%S)\n", Fcb->PipeName.Buffer);
+ DPRINT("Write Waiting for buffer space (%S)\n", Fcb->PipeName.Buffer);
Status = KeWaitForSingleObject(&Ccb->WriteEvent,
UserRequest,
Irp->RequestorMode,
FALSE,
NULL);
- DPRINT("Finished waiting (%S)! Status: %x\n", Fcb->PipeName.Buffer, Status);
-
+ DPRINT("Write Finished waiting (%S)! Status: %x\n", Fcb->PipeName.Buffer, Status);
+ ExAcquireFastMutex(&ReaderCcb->DataListLock);
if ((Status == STATUS_USER_APC) || (Status == STATUS_KERNEL_APC))
{
Status = STATUS_CANCELLED;
@@ -777,8 +795,6 @@
// ExReleaseFastMutex(&ReaderCcb->DataListLock);
goto done;
}
-
- ExAcquireFastMutex(&ReaderCcb->DataListLock);
}
if (Fcb->WriteMode == FILE_PIPE_BYTE_STREAM_MODE)
@@ -825,21 +841,41 @@
{
/* For Message Type Pipe, the Pipes memory will be used to store the size of each message */
/* FIXME: Check and verify ReadMode ByteStream */
- DPRINT("Message mode\n");
+
if (Length > 0)
{
- CopyLength = min(Length, ReaderCcb->WriteQuotaAvailable);
+ /* Verify the WritePtr is still inside the buffer */
+ if (((ULONG)ReaderCcb->WritePtr > ((ULONG)ReaderCcb->Data + (ULONG)ReaderCcb->MaxDataLength)) ||
+ ((ULONG)ReaderCcb->WritePtr < (ULONG)ReaderCcb->Data))
+ {
+ DPRINT1("NPFS is writing out of its buffer. Report to developer!\n");
+ DPRINT1("ReaderCcb->WritePtr %x, ReaderCcb->Data %x, ReaderCcb->MaxDataLength%d\n",
+ ReaderCcb->WritePtr,ReaderCcb->Data,ReaderCcb->MaxDataLength);
+ ASSERT(FALSE);
+ }
+
+ CopyLength = min(Length, ReaderCcb->WriteQuotaAvailable - sizeof(ULONG));
+
/* First Copy the Length of the message into the pipes buffer */
memcpy(ReaderCcb->WritePtr, &CopyLength, sizeof(CopyLength));
+
/* Now the user buffer itself */
memcpy((PVOID)((ULONG)ReaderCcb->WritePtr+ sizeof(CopyLength)), Buffer, CopyLength);
+
/* Update the write pointer */
ReaderCcb->WritePtr = (PVOID)((ULONG)ReaderCcb->WritePtr + sizeof(CopyLength) + CopyLength);
Information += CopyLength;
ReaderCcb->ReadDataAvailable += CopyLength;
- ReaderCcb->WriteQuotaAvailable -= CopyLength;
+
+ ReaderCcb->WriteQuotaAvailable -= (CopyLength + sizeof(ULONG));
+
+ if ((ULONG)ReaderCcb->WriteQuotaAvailable > (ULONG)ReaderCcb->MaxDataLength)
+ {
+ DPRINT1("QuotaAvailable is greater than buffer size!\n");
+ ASSERT(FALSE);
+ }
}
if (Information > 0)
More information about the Ros-diffs
mailing list