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/cr... ============================================================================== --- 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/di... ============================================================================== --- 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/fs... ============================================================================== --- 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/np... ============================================================================== --- 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/np... ============================================================================== --- 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/rw... ============================================================================== --- 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);