[tfaber] 56950: [NPFS] - Keep a reference count for FCBs to prevent race conditions when closing multiple handles to the same pipe concurrently - Acquire the pipe list lock in NpfsQueryDirectory to...
Author: tfaber Date: Mon Jul 23 13:24:48 2012 New Revision: 56950
URL: http://svn.reactos.org/svn/reactos?rev=56950&view=rev Log: [NPFS] - Keep a reference count for FCBs to prevent race conditions when closing multiple handles to the same pipe concurrently - Acquire the pipe list lock in NpfsQueryDirectory to guard against concurrent deletion of pipes See issue #7205 for more details.
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
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] Mon Jul 23 13:24:48 2012 @@ -17,17 +17,20 @@
/* FUNCTIONS *****************************************************************/
-static VOID -NpfsDeleteFcb(PNPFS_FCB Fcb) +NpfsDereferenceFcb(PNPFS_FCB Fcb) { PNPFS_VCB Vcb = Fcb->Vcb;
KeLockMutex(&Vcb->PipeListLock); - RemoveEntryList(&Fcb->PipeListEntry); + if (InterlockedDecrement(&Fcb->RefCount) == 0) + { + DPRINT("NpfsDereferenceFcb. Deleting %p\n", Fcb); + RemoveEntryList(&Fcb->PipeListEntry); + RtlFreeUnicodeString(&Fcb->PipeName); + ExFreePoolWithTag(Fcb, TAG_NPFS_FCB); + } KeUnlockMutex(&Vcb->PipeListLock); - RtlFreeUnicodeString(&Fcb->PipeName); - ExFreePoolWithTag(Fcb, TAG_NPFS_FCB); }
static @@ -103,6 +106,7 @@ TRUE) == 0) { DPRINT("<%wZ> = <%wZ>\n", PipeName, &Fcb->PipeName); + (VOID)InterlockedIncrement(&Fcb->RefCount); return Fcb; }
@@ -337,6 +341,7 @@ { DPRINT("No memory!\n"); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -365,6 +370,7 @@ DPRINT("No memory!\n"); NpfsDereferenceCcb(ClientCcb); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -437,6 +443,7 @@
NpfsDereferenceCcb(ClientCcb); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_OBJECT_PATH_NOT_FOUND; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_OBJECT_PATH_NOT_FOUND; @@ -460,8 +467,8 @@ }
NpfsDereferenceCcb(ClientCcb); - KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_UNSUCCESSFUL; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_UNSUCCESSFUL; @@ -510,7 +517,6 @@ PNPFS_FCB Fcb; PNPFS_CCB Ccb; PNAMED_PIPE_CREATE_PARAMETERS Buffer; - BOOLEAN NewPipe = FALSE;
DPRINT("NpfsCreateNamedPipe(DeviceObject %p Irp %p)\n", DeviceObject, Irp);
@@ -549,6 +555,7 @@ if (Fcb->CurrentInstances >= Fcb->MaximumInstances) { DPRINT("Out of instances.\n"); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_INSTANCE_NOT_AVAILABLE; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_INSTANCE_NOT_AVAILABLE; @@ -559,6 +566,7 @@ Fcb->PipeType != Buffer->NamedPipeType) { DPRINT("Asked for invalid pipe mode.\n"); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_ACCESS_DENIED; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_ACCESS_DENIED; @@ -566,7 +574,6 @@ } else { - NewPipe = TRUE; Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); if (Fcb == NULL) { @@ -579,6 +586,7 @@
Fcb->Type = FCB_PIPE; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Fcb->PipeName.Length = FileObject->FileName.Length; Fcb->PipeName.MaximumLength = Fcb->PipeName.Length + sizeof(UNICODE_NULL); Fcb->PipeName.Buffer = ExAllocatePoolWithTag(NonPagedPool, @@ -678,11 +686,7 @@ Ccb = NpfsAllocateCcb(CCB_PIPE, Fcb); if (Ccb == NULL) { - if (NewPipe) - { - NpfsDeleteFcb(Fcb); - } - + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -698,11 +702,7 @@ if (Ccb->Data == NULL) { NpfsDereferenceCcb(Ccb); - - if (NewPipe) - { - NpfsDeleteFcb(Fcb); - } + NpfsDereferenceFcb(Fcb);
Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); @@ -1004,12 +1004,8 @@
KeUnlockMutex(&Fcb->CcbListLock);
- if (IsListEmpty(&Fcb->ServerCcbListHead) && - IsListEmpty(&Fcb->ClientCcbListHead)) - { - NpfsDeleteFcb(Fcb); - FileObject->FsContext = NULL; - } + NpfsDereferenceFcb(Fcb); + FileObject->FsContext = NULL;
Irp->IoStatus.Status = STATUS_SUCCESS; Irp->IoStatus.Information = 0;
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] Mon Jul 23 13:24:48 2012 @@ -144,6 +144,7 @@ PipeIndex = 0;
Vcb = Ccb->Fcb->Vcb; + KeLockMutex(&Vcb->PipeListLock); CurrentEntry = Vcb->PipeListHead.Flink; while (CurrentEntry != &Vcb->PipeListHead && Status == STATUS_SUCCESS) @@ -252,11 +253,17 @@
/* Leave, if there is no space left in the buffer */ if (Status == STATUS_BUFFER_OVERFLOW) + { + KeUnlockMutex(&Vcb->PipeListLock); return Status; + }
/* Leave, if we should return only one entry */ if (Stack->Flags & SL_RETURN_SINGLE_ENTRY) + { + KeUnlockMutex(&Vcb->PipeListLock); return STATUS_SUCCESS; + }
/* Store the current offset for the next round */ LastOffset = CurrentOffset; @@ -270,6 +277,7 @@
CurrentEntry = CurrentEntry->Flink; } + KeUnlockMutex(&Vcb->PipeListLock);
/* Return STATUS_NO_MORE_FILES if no matching pipe name was found */ if (CurrentOffset == 0)
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] Mon Jul 23 13:24:48 2012 @@ -375,7 +375,7 @@ { /* found a listening server CCB */ DPRINT("Listening server CCB found -- connecting\n"); - + NpfsDereferenceFcb(Fcb); return STATUS_SUCCESS; }
@@ -402,6 +402,7 @@ /* Wait forever */ TimeOut = NULL; } + NpfsDereferenceFcb(Fcb);
Status = KeWaitForSingleObject(&Ccb->ConnectEvent, UserRequest, @@ -507,7 +508,9 @@ { /* found a listening server CCB */ DPRINT("Listening server CCB found -- connecting\n"); - +#ifdef USING_PROPER_NPFS_WAIT_SEMANTICS + NpfsDereferenceFcb(Fcb); +#endif return STATUS_SUCCESS; }
@@ -521,6 +524,9 @@ TimeOut = WaitPipe->Timeout; else TimeOut = Fcb->TimeOut; +#ifdef USING_PROPER_NPFS_WAIT_SEMANTICS + NpfsDereferenceFcb(Fcb); +#endif
/* Wait for one */ Status = KeWaitForSingleObject(&Ccb->ConnectEvent,
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] Mon Jul 23 13:24:48 2012 @@ -89,12 +89,14 @@ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); Fcb->Type = FCB_DEVICE; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Vcb->DeviceFcb = Fcb;
/* Create the root directory FCB */ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); Fcb->Type = FCB_DIRECTORY; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Vcb->RootFcb = Fcb;
return STATUS_SUCCESS;
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] Mon Jul 23 13:24:48 2012 @@ -51,6 +51,7 @@ { FCB_TYPE Type; PNPFS_VCB Vcb; + volatile LONG RefCount; UNICODE_STRING PipeName; LIST_ENTRY PipeListEntry; KMUTEX CcbListLock; @@ -83,8 +84,7 @@ LIST_ENTRY CcbListEntry; CCB_TYPE Type; PNPFS_FCB Fcb; - - PFILE_OBJECT FileObject; + PFILE_OBJECT FileObject;
struct _NPFS_CCB* OtherSide; struct ETHREAD *Thread; @@ -190,6 +190,9 @@ DriverEntry(PDRIVER_OBJECT DriverObject, PUNICODE_STRING RegistryPath);
+VOID +NpfsDereferenceFcb(PNPFS_FCB Fcb); + PNPFS_FCB NpfsFindPipe(PNPFS_VCB Vcb, PUNICODE_STRING PipeName);
-
tfaber＠svn.reactos.org