6 Jul
2011
6 Jul
'11
6:14 p.m.
Author: tkreuzer
Date: Wed Jul 6 18:14:42 2011
New Revision: 52547
URL: http://svn.reactos.org/svn/reactos?rev=52547&view=rev
Log:
[NPFS]
Fix 2nd stage pool corruption.
CCBs are organized in a linked list in the corresponding Fcb, which is protected by
Fcb->CcbListLock. They are also linked together server <-> client. This way they
can be referenced without holding the lock. This lead to a race condition where a
CCB's link was modified after the CCB was deleted. Fix this by using a reference
counter and adding appropriate functions. Also make use of pool tags.
Modified:
trunk/reactos/drivers/filesystems/npfs/create.c
trunk/reactos/drivers/filesystems/npfs/dirctl.c
trunk/reactos/drivers/filesystems/npfs/fsctrl.c
trunk/reactos/drivers/filesystems/npfs/npfs.c
trunk/reactos/drivers/filesystems/npfs/npfs.h
trunk/reactos/drivers/filesystems/npfs/rw.c
Modified: trunk/reactos/drivers/filesystems/npfs/create.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/c…
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/create.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/create.c [iso-8859-1] Wed Jul 6 18:14:42 2011
@@ -16,6 +16,76 @@
//#define USING_PROPER_NPFS_WAIT_SEMANTICS
/* FUNCTIONS *****************************************************************/
+
+static
+VOID
+NpfsDeleteFcb(PNPFS_FCB Fcb)
+{
+ PNPFS_VCB Vcb = Fcb->Vcb;
+
+ KeLockMutex(&Vcb->PipeListLock);
+ RemoveEntryList(&Fcb->PipeListEntry);
+ KeUnlockMutex(&Vcb->PipeListLock);
+ RtlFreeUnicodeString(&Fcb->PipeName);
+ ExFreePoolWithTag(Fcb, TAG_NPFS_FCB);
+}
+
+static
+PNPFS_CCB
+NpfsAllocateCcb(CCB_TYPE Type, PNPFS_FCB Fcb)
+{
+ PNPFS_CCB Ccb;
+
+ Ccb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_CCB), TAG_NPFS_CCB);
+ if (!Ccb)
+ {
+ return NULL;
+ }
+
+ RtlZeroMemory(Ccb, sizeof(NPFS_CCB));
+
+ Ccb->RefCount = 1;
+ Ccb->Type = Type;
+ Ccb->Fcb = Fcb;
+ Ccb->OtherSide = NULL;
+
+ return Ccb;
+}
+
+static
+VOID
+NpfsReferenceCcb(PNPFS_CCB Ccb)
+{
+ ASSERT(Ccb->RefCount > 0);
+ InterlockedIncrement((PLONG)&Ccb->RefCount);
+}
+
+static
+VOID
+NpfsDereferenceCcb(PNPFS_CCB Ccb)
+{
+ /* Decrement reference count */
+ ASSERT(Ccb->RefCount > 0);
+ if (InterlockedDecrement((PLONG)&Ccb->RefCount) == 0)
+ {
+ /* Its zero, delete CCB */
+ ExFreePoolWithTag(Ccb, TAG_NPFS_CCB);
+ }
+}
+
+static
+VOID
+NpfsCcbSetOtherSide(PNPFS_CCB Ccb, PNPFS_CCB OtherSide)
+{
+ /* Dereference old other side */
+ if (Ccb->OtherSide) NpfsDereferenceCcb(Ccb->OtherSide);
+
+ /* Reference the new other side */
+ if (OtherSide) NpfsReferenceCcb(OtherSide);
+
+ /* Set new value */
+ Ccb->OtherSide = OtherSide;
+}
PNPFS_FCB
NpfsFindPipe(PNPFS_VCB Vcb,
@@ -114,17 +184,12 @@
DPRINT("NpfsOpenFileSystem()\n");
- Ccb = ExAllocatePool(NonPagedPool, sizeof(NPFS_CCB));
+ Ccb = NpfsAllocateCcb(CCB_DEVICE, Fcb);
if (Ccb == NULL)
{
IoStatus->Status = STATUS_NO_MEMORY;
return;
}
-
- RtlZeroMemory(Ccb, sizeof(NPFS_CCB));
-
- Ccb->Type = CCB_DEVICE;
- Ccb->Fcb = Fcb;
FileObject->FsContext = Fcb;
FileObject->FsContext2 = Ccb;
@@ -145,17 +210,12 @@
DPRINT("NpfsOpenRootDirectory()\n");
- Ccb = ExAllocatePool(NonPagedPool, sizeof(NPFS_CCB));
+ Ccb = NpfsAllocateCcb(CCB_DIRECTORY, Fcb);
if (Ccb == NULL)
{
IoStatus->Status = STATUS_NO_MEMORY;
return;
}
-
- RtlZeroMemory(Ccb, sizeof(NPFS_CCB));
-
- Ccb->Type = CCB_DIRECTORY;
- Ccb->Fcb = Fcb;
FileObject->FsContext = Fcb;
FileObject->FsContext2 = Ccb;
@@ -244,8 +304,7 @@
* Step 1. Find the pipe we're trying to open.
*/
KeLockMutex(&Vcb->PipeListLock);
- Fcb = NpfsFindPipe(Vcb,
- &FileObject->FileName);
+ Fcb = NpfsFindPipe(Vcb, &FileObject->FileName);
if (Fcb == NULL)
{
/* Not found, bail out with error. */
@@ -267,7 +326,7 @@
/*
* Step 2. Create the client CCB.
*/
- ClientCcb = ExAllocatePool(NonPagedPool, sizeof(NPFS_CCB));
+ ClientCcb = NpfsAllocateCcb(CCB_PIPE, Fcb);
if (ClientCcb == NULL)
{
DPRINT("No memory!\n");
@@ -277,11 +336,8 @@
return STATUS_NO_MEMORY;
}
- ClientCcb->Type = CCB_PIPE;
ClientCcb->Thread = (struct ETHREAD *)Irp->Tail.Overlay.Thread;
- ClientCcb->Fcb = Fcb;
ClientCcb->PipeEnd = FILE_PIPE_CLIENT_END;
- ClientCcb->OtherSide = NULL;
#ifndef USING_PROPER_NPFS_WAIT_SEMANTICS
ClientCcb->PipeState = SpecialAccess ? 0 : FILE_PIPE_DISCONNECTED_STATE;
#else
@@ -294,11 +350,13 @@
/* Initialize data list. */
if (Fcb->OutboundQuota)
{
- ClientCcb->Data = ExAllocatePool(PagedPool, Fcb->OutboundQuota);
+ ClientCcb->Data = ExAllocatePoolWithTag(PagedPool,
+ Fcb->OutboundQuota,
+ TAG_NPFS_CCB_DATA);
if (ClientCcb->Data == NULL)
{
DPRINT("No memory!\n");
- ExFreePool(ClientCcb);
+ NpfsDereferenceCcb(ClientCcb);
KeUnlockMutex(&Fcb->CcbListLock);
Irp->IoStatus.Status = STATUS_NO_MEMORY;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -367,10 +425,10 @@
DPRINT("No listening server CCB found!\n");
if (ClientCcb->Data)
{
- ExFreePool(ClientCcb->Data);
+ ExFreePoolWithTag(ClientCcb->Data, TAG_NPFS_CCB_DATA);
}
- ExFreePool(ClientCcb);
+ NpfsDereferenceCcb(ClientCcb);
KeUnlockMutex(&Fcb->CcbListLock);
Irp->IoStatus.Status = STATUS_OBJECT_PATH_NOT_FOUND;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -391,10 +449,10 @@
if (ClientCcb->Data)
{
- ExFreePool(ClientCcb->Data);
- }
-
- ExFreePool(ClientCcb);
+ ExFreePoolWithTag(ClientCcb->Data, TAG_NPFS_CCB_DATA);
+ }
+
+ NpfsDereferenceCcb(ClientCcb);
KeUnlockMutex(&Fcb->CcbListLock);
Irp->IoStatus.Status = STATUS_UNSUCCESSFUL;
@@ -413,8 +471,8 @@
/* Connect to listening server side */
if (ServerCcb)
{
- ClientCcb->OtherSide = ServerCcb;
- ServerCcb->OtherSide = ClientCcb;
+ NpfsCcbSetOtherSide(ClientCcb, ServerCcb);
+ NpfsCcbSetOtherSide(ServerCcb, ClientCcb);
ClientCcb->PipeState = FILE_PIPE_CONNECTED_STATE;
ServerCcb->PipeState = FILE_PIPE_CONNECTED_STATE;
KeSetEvent(&ServerCcb->ConnectEvent, IO_NO_INCREMENT, FALSE);
@@ -467,23 +525,12 @@
return STATUS_INVALID_PARAMETER;
}
- Ccb = ExAllocatePool(NonPagedPool, sizeof(NPFS_CCB));
- if (Ccb == NULL)
- {
- Irp->IoStatus.Status = STATUS_NO_MEMORY;
- IoCompleteRequest(Irp, IO_NO_INCREMENT);
- return STATUS_NO_MEMORY;
- }
-
- Ccb->Type = CCB_PIPE;
- Ccb->Thread = (struct ETHREAD *)Irp->Tail.Overlay.Thread;
KeLockMutex(&Vcb->PipeListLock);
/*
* First search for existing Pipe with the same name.
*/
- Fcb = NpfsFindPipe(Vcb,
- &FileObject->FileName);
+ Fcb = NpfsFindPipe(Vcb, &FileObject->FileName);
if (Fcb != NULL)
{
/*
@@ -495,7 +542,6 @@
if (Fcb->CurrentInstances >= Fcb->MaximumInstances)
{
DPRINT("Out of instances.\n");
- ExFreePool(Ccb);
Irp->IoStatus.Status = STATUS_INSTANCE_NOT_AVAILABLE;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
return STATUS_INSTANCE_NOT_AVAILABLE;
@@ -506,7 +552,6 @@
Fcb->PipeType != Buffer->NamedPipeType)
{
DPRINT("Asked for invalid pipe mode.\n");
- ExFreePool(Ccb);
Irp->IoStatus.Status = STATUS_ACCESS_DENIED;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
return STATUS_ACCESS_DENIED;
@@ -515,11 +560,10 @@
else
{
NewPipe = TRUE;
- Fcb = ExAllocatePool(NonPagedPool, sizeof(NPFS_FCB));
+ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB);
if (Fcb == NULL)
{
KeUnlockMutex(&Vcb->PipeListLock);
- ExFreePool(Ccb);
Irp->IoStatus.Status = STATUS_NO_MEMORY;
Irp->IoStatus.Information = 0;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -530,12 +574,13 @@
Fcb->Vcb = Vcb;
Fcb->PipeName.Length = FileObject->FileName.Length;
Fcb->PipeName.MaximumLength = Fcb->PipeName.Length + sizeof(UNICODE_NULL);
- Fcb->PipeName.Buffer = ExAllocatePool(NonPagedPool,
Fcb->PipeName.MaximumLength);
+ Fcb->PipeName.Buffer = ExAllocatePoolWithTag(NonPagedPool,
+ Fcb->PipeName.MaximumLength,
+ TAG_NPFS_NAMEBLOCK);
if (Fcb->PipeName.Buffer == NULL)
{
KeUnlockMutex(&Vcb->PipeListLock);
- ExFreePool(Fcb);
- ExFreePool(Ccb);
+ ExFreePoolWithTag(Fcb, TAG_NPFS_FCB);
Irp->IoStatus.Status = STATUS_NO_MEMORY;
Irp->IoStatus.Information = 0;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -623,20 +668,33 @@
KeUnlockMutex(&Vcb->PipeListLock);
}
+ Ccb = NpfsAllocateCcb(CCB_PIPE, Fcb);
+ if (Ccb == NULL)
+ {
+ if (NewPipe)
+ {
+ NpfsDeleteFcb(Fcb);
+ }
+
+ Irp->IoStatus.Status = STATUS_NO_MEMORY;
+ IoCompleteRequest(Irp, IO_NO_INCREMENT);
+ return STATUS_NO_MEMORY;
+ }
+
+ Ccb->Thread = (struct ETHREAD *)Irp->Tail.Overlay.Thread;
+
if (Fcb->InboundQuota)
{
- Ccb->Data = ExAllocatePool(PagedPool, Fcb->InboundQuota);
+ Ccb->Data = ExAllocatePoolWithTag(PagedPool,
+ Fcb->InboundQuota,
+ TAG_NPFS_CCB_DATA);
if (Ccb->Data == NULL)
{
- ExFreePool(Ccb);
+ NpfsDereferenceCcb(Ccb);
if (NewPipe)
{
- KeLockMutex(&Vcb->PipeListLock);
- RemoveEntryList(&Fcb->PipeListEntry);
- KeUnlockMutex(&Vcb->PipeListLock);
- RtlFreeUnicodeString(&Fcb->PipeName);
- ExFreePool(Fcb);
+ NpfsDeleteFcb(Fcb);
}
Irp->IoStatus.Status = STATUS_NO_MEMORY;
@@ -662,7 +720,6 @@
Ccb->Fcb = Fcb;
Ccb->PipeEnd = FILE_PIPE_SERVER_END;
Ccb->PipeState = FILE_PIPE_LISTENING_STATE;
- Ccb->OtherSide = NULL;
DPRINT("CCB: %p\n", Ccb);
@@ -753,6 +810,8 @@
if ((Ccb->PipeState == FILE_PIPE_CONNECTED_STATE) && (Ccb->OtherSide))
{
OtherSide = Ccb->OtherSide;
+ ASSERT(OtherSide->OtherSide == Ccb);
+
/* Lock the server first */
if (Server)
{
@@ -764,7 +823,11 @@
ExAcquireFastMutex(&OtherSide->DataListLock);
ExAcquireFastMutex(&Ccb->DataListLock);
}
- OtherSide->OtherSide = NULL;
+
+ /* Unlink FCBs */
+ NpfsCcbSetOtherSide(OtherSide, NULL);
+ NpfsCcbSetOtherSide(Ccb, NULL);
+
/*
* Signaling the write event. If is possible that an other
* thread waits for an empty buffer.
@@ -824,7 +887,7 @@
ExAcquireFastMutex(&Ccb->DataListLock);
if (Ccb->Data)
{
- ExFreePool(Ccb->Data);
+ ExFreePoolWithTag(Ccb->Data, TAG_NPFS_CCB_DATA);
Ccb->Data = NULL;
Ccb->ReadPtr = NULL;
Ccb->WritePtr = NULL;
@@ -871,7 +934,7 @@
{
DPRINT("Closing the file system!\n");
- ExFreePool(Ccb);
+ NpfsDereferenceCcb(Ccb);
FileObject->FsContext = NULL;
FileObject->FsContext2 = NULL;
@@ -886,9 +949,10 @@
DPRINT("Closing the root directory!\n");
if (Ccb->u.Directory.SearchPattern.Buffer != NULL)
- ExFreePool(Ccb->u.Directory.SearchPattern.Buffer);
-
- ExFreePool(Ccb);
+ ExFreePoolWithTag(Ccb->u.Directory.SearchPattern.Buffer,
+ TAG_NPFS_NAMEBLOCK);
+
+ NpfsDereferenceCcb(Ccb);
FileObject->FsContext = NULL;
FileObject->FsContext2 = NULL;
@@ -920,8 +984,9 @@
/* Disconnect the pipes */
if (Ccb->OtherSide)
{
- Ccb->OtherSide->OtherSide = NULL;
- Ccb->OtherSide = NULL;
+ ASSERT(Ccb->OtherSide->OtherSide == Ccb);
+ NpfsCcbSetOtherSide(Ccb->OtherSide, NULL);
+ NpfsCcbSetOtherSide(Ccb, NULL);
}
ASSERT(Ccb->PipeState == FILE_PIPE_CLOSING_STATE);
@@ -930,18 +995,14 @@
RemoveEntryList(&Ccb->CcbListEntry);
- ExFreePool(Ccb);
+ NpfsDereferenceCcb(Ccb);
KeUnlockMutex(&Fcb->CcbListLock);
if (IsListEmpty(&Fcb->ServerCcbListHead) &&
IsListEmpty(&Fcb->ClientCcbListHead))
{
- KeLockMutex(&Vcb->PipeListLock);
- RemoveEntryList(&Fcb->PipeListEntry);
- KeUnlockMutex(&Vcb->PipeListLock);
- RtlFreeUnicodeString(&Fcb->PipeName);
- ExFreePool(Fcb);
+ NpfsDeleteFcb(Fcb);
FileObject->FsContext = NULL;
}
Modified: trunk/reactos/drivers/filesystems/npfs/dirctl.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/d…
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/dirctl.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/dirctl.c [iso-8859-1] Wed Jul 6 18:14:42 2011
@@ -71,7 +71,9 @@
if (SearchPattern != NULL)
{
Ccb->u.Directory.SearchPattern.Buffer =
- ExAllocatePool(NonPagedPool, SearchPattern->Length + sizeof(WCHAR));
+ ExAllocatePoolWithTag(NonPagedPool,
+ SearchPattern->Length + sizeof(WCHAR),
+ TAG_NPFS_NAMEBLOCK);
if (Ccb->u.Directory.SearchPattern.Buffer == NULL)
{
return STATUS_INSUFFICIENT_RESOURCES;
@@ -87,7 +89,9 @@
else
{
Ccb->u.Directory.SearchPattern.Buffer =
- ExAllocatePool(NonPagedPool, 2 * sizeof(WCHAR));
+ ExAllocatePoolWithTag(NonPagedPool,
+ 2 * sizeof(WCHAR),
+ TAG_NPFS_NAMEBLOCK);
if (Ccb->u.Directory.SearchPattern.Buffer == NULL)
{
return STATUS_INSUFFICIENT_RESOURCES;
Modified: trunk/reactos/drivers/filesystems/npfs/fsctrl.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/f…
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/fsctrl.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/fsctrl.c [iso-8859-1] Wed Jul 6 18:14:42 2011
@@ -441,7 +441,9 @@
/* Calculate the pipe name length and allocate the buffer */
PipeName.Length = WaitPipe->NameLength + sizeof(WCHAR);
PipeName.MaximumLength = PipeName.Length + sizeof(WCHAR);
- PipeName.Buffer = ExAllocatePool(NonPagedPool, PipeName.MaximumLength);
+ PipeName.Buffer = ExAllocatePoolWithTag(NonPagedPool,
+ PipeName.MaximumLength,
+ TAG_NPFS_NAMEBLOCK);
if (PipeName.Buffer == NULL)
{
DPRINT1("Could not allocate memory for the pipe name!\n");
@@ -471,7 +473,7 @@
KeUnlockMutex(&Vcb->PipeListLock);
/* Release the pipe name buffer */
- ExFreePool(PipeName.Buffer);
+ ExFreePoolWithTag(PipeName.Buffer, TAG_NPFS_NAMEBLOCK);
/* Fail if not pipe was found */
if (Fcb == NULL)
Modified: trunk/reactos/drivers/filesystems/npfs/npfs.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/n…
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/npfs.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/npfs.c [iso-8859-1] Wed Jul 6 18:14:42 2011
@@ -86,13 +86,13 @@
Vcb->MaxQuota = 64 * PAGE_SIZE;
/* Create the device FCB */
- Fcb = ExAllocatePool(NonPagedPool, sizeof(NPFS_FCB));
+ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB);
Fcb->Type = FCB_DEVICE;
Fcb->Vcb = Vcb;
Vcb->DeviceFcb = Fcb;
/* Create the root directory FCB */
- Fcb = ExAllocatePool(NonPagedPool, sizeof(NPFS_FCB));
+ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB);
Fcb->Type = FCB_DIRECTORY;
Fcb->Vcb = Vcb;
Vcb->RootFcb = Fcb;
Modified: trunk/reactos/drivers/filesystems/npfs/npfs.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/n…
==============================================================================
--- trunk/reactos/drivers/filesystems/npfs/npfs.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/filesystems/npfs/npfs.h [iso-8859-1] Wed Jul 6 18:14:42 2011
@@ -4,6 +4,12 @@
#include <ntifs.h>
#include <ndk/iotypes.h>
#include <pseh/pseh2.h>
+
+#define TAG_NPFS_CCB 'cFpN'
+#define TAG_NPFS_CCB_DATA 'iFpN' /* correct? */
+#define TAG_NPFS_FCB 'FFpN'
+#define TAG_NPFS_NAMEBLOCK 'nFpN'
+#define TAG_NPFS_THREAD_CONTEXT 'tFpN'
#define ROUND_DOWN(n, align) \
(((ULONG)n) & ~((align) - 1l))
@@ -87,6 +93,7 @@
ULONG PipeState;
ULONG ReadDataAvailable;
ULONG WriteQuotaAvailable;
+ ULONG RefCount;
LIST_ENTRY ReadRequestListHead;
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] Wed Jul 6 18:14:42 2011
@@ -194,12 +194,11 @@
{
/* it exist an other thread with empty wait slots, we can remove our thread
from the list */
RemoveEntryList(&ThreadContext->ListEntry);
- ThreadContext->Vcb->EmptyWaiterCount -= MAXIMUM_WAIT_OBJECTS - 1;
+ ExFreePoolWithTag(ThreadContext, TAG_NPFS_THREAD_CONTEXT);
KeUnlockMutex(&ThreadContext->Vcb->PipeListLock);
break;
}
}
- ExFreePool(ThreadContext);
}
static NTSTATUS
@@ -234,7 +233,9 @@
if (ListEntry == &Vcb->ThreadListHead)
{
- ThreadContext = ExAllocatePool(NonPagedPool, sizeof(NPFS_THREAD_CONTEXT));
+ ThreadContext = ExAllocatePoolWithTag(NonPagedPool,
+ sizeof(NPFS_THREAD_CONTEXT),
+ TAG_NPFS_THREAD_CONTEXT);
if (ThreadContext == NULL)
{
KeUnlockMutex(&Vcb->PipeListLock);
@@ -257,7 +258,7 @@
(PVOID)ThreadContext);
if (!NT_SUCCESS(Status))
{
- ExFreePool(ThreadContext);
+ ExFreePoolWithTag(ThreadContext, TAG_NPFS_THREAD_CONTEXT);
KeUnlockMutex(&Vcb->PipeListLock);
return Status;
}
@@ -638,7 +639,7 @@
Buffer = (PVOID)((ULONG_PTR)Buffer + CopyLength);
Length -= CopyLength;
}
- else
+ else
{
KeResetEvent(&Ccb->ReadEvent);