[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.
11 Jan
2009
11 Jan
'09
2:28 p.m.
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/r…
==============================================================================
--- 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)
5942
days inactive
5942
days old
0 comments
1 participants
participants (1)
-
mjmartin＠svn.reactos.org