[ion] 60027: [NPFS-NEW]: Multiple code fixes to almost all code paths. Mainly a collection of small bugs, structural errors, logic flaws, off-by-ones, etc. But they were enough to make the driver e...
Author: ion Date: Wed Sep 11 02:04:17 2013 New Revision: 60027
URL: http://svn.reactos.org/svn/reactos?rev=60027&view=rev Log: [NPFS-NEW]: Multiple code fixes to almost all code paths. Mainly a collection of small bugs, structural errors, logic flaws, off-by-ones, etc. But they were enough to make the driver essentially useless. Now Thomas's kmtests NpfsCreate and NpfsConnect return absolutely zero failures. On to NpfsReadWrite....
Modified: trunk/reactos/drivers/filesystems/npfs_new/create.c trunk/reactos/drivers/filesystems/npfs_new/datasup.c trunk/reactos/drivers/filesystems/npfs_new/fileinfo.c trunk/reactos/drivers/filesystems/npfs_new/npfs.h trunk/reactos/drivers/filesystems/npfs_new/statesup.c trunk/reactos/drivers/filesystems/npfs_new/strucsup.c
Modified: trunk/reactos/drivers/filesystems/npfs_new/create.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/create.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/create.c [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -14,6 +14,7 @@ ListHead = &Dcb->NotifyList; for (i = 0; i < 2; i++) { + ASSERT(IsListEmpty(ListHead)); while (!IsListEmpty(ListHead)) { NextEntry = RemoveHeadList(ListHead); @@ -47,7 +48,7 @@ NpSetFileObject(FileObject, NpVcb, NULL, FALSE); ++NpVcb->ReferenceCount;
- Status.Information = 1; + Status.Information = FILE_OPENED; Status.Status = STATUS_SUCCESS; return Status; } @@ -69,7 +70,7 @@ NpSetFileObject(FileObject, Dcb, Ccb, FALSE); ++Dcb->CurrentInstances;
- Status.Information = 1; + Status.Information = FILE_OPENED; Status.Status = STATUS_SUCCESS; } else @@ -130,7 +131,7 @@ if (AccessGranted) { AccessState->PreviouslyGrantedAccess |= GrantedAccess; - AccessState->RemainingDesiredAccess &= ~(GrantedAccess | 0x2000000); + AccessState->RemainingDesiredAccess &= ~(GrantedAccess | MAXIMUM_ALLOWED); }
ObjectTypeName.Buffer = L"NamedPipe"; @@ -147,14 +148,14 @@ SeUnlockSubjectContext(SubjectSecurityContext); if (!AccessGranted) return IoStatus;
- if (((GrantedAccess & 1) && (NamedPipeConfiguration == FILE_PIPE_INBOUND)) || - ((GrantedAccess & 2) && (NamedPipeConfiguration == FILE_PIPE_OUTBOUND))) + if (((AccessGranted & FILE_READ_DATA) && (NamedPipeConfiguration == FILE_PIPE_INBOUND)) || + ((AccessGranted & FILE_WRITE_DATA) && (NamedPipeConfiguration == FILE_PIPE_OUTBOUND))) { IoStatus.Status = STATUS_ACCESS_DENIED; return IoStatus; }
- if (!(GrantedAccess & 3)) SecurityQos = NULL; + if (!(GrantedAccess & (FILE_READ_DATA | FILE_WRITE_DATA))) SecurityQos = NULL;
ListHead = &Fcb->CcbList; NextEntry = ListHead->Flink; @@ -185,7 +186,7 @@ Ccb->ClientSession = NULL; Ccb->Process = IoThreadToProcess(Thread);
- IoStatus.Information = 1; + IoStatus.Information = FILE_OPENED; IoStatus.Status = STATUS_SUCCESS; return IoStatus; } @@ -205,7 +206,9 @@ PNP_DCB Dcb; ACCESS_MASK DesiredAccess; LIST_ENTRY List; + PLIST_ENTRY NextEntry, ThisEntry; UNICODE_STRING Prefix; + PIRP ListIrp;
InitializeListHead(&List); IoStack = (PEXTENDED_IO_STACK_LOCATION)IoGetCurrentIrpStackLocation(Irp); @@ -235,20 +238,22 @@
if (FileName.Length) { - if ((FileName.Length == sizeof(WCHAR)) && + if ((FileName.Length == sizeof(OBJ_NAME_PATH_SEPARATOR)) && (FileName.Buffer[0] == OBJ_NAME_PATH_SEPARATOR) && !(RelatedFileObject)) { Irp->IoStatus = NpOpenNamedPipeRootDirectory(NpVcb->RootDcb, - FileObject, - DesiredAccess, - &List); + FileObject, + DesiredAccess, + &List); + goto Quickie; } } else if (!(RelatedFileObject) || (Type == NPFS_NTC_VCB)) { Irp->IoStatus = NpOpenNamedPipeFileSystem(FileObject, DesiredAccess); + goto Quickie; } else if (Type == NPFS_NTC_ROOT_DCB) { @@ -256,77 +261,94 @@ FileObject, DesiredAccess, &List); - } - else - { - // Status = NpTranslateAlias(&FileName);; // ignore this for now - // if (!NT_SUCCESS(Status)) goto Quickie; - - if (RelatedFileObject) - { - if (Type == NPFS_NTC_ROOT_DCB) + goto Quickie; + } + + // Status = NpTranslateAlias(&FileName);; // ignore this for now + // if (!NT_SUCCESS(Status)) goto Quickie; + if (RelatedFileObject) + { + if (Type == NPFS_NTC_ROOT_DCB) + { + Dcb = (PNP_DCB)Ccb; + Irp->IoStatus.Status = NpFindRelativePrefix(Dcb, + &FileName, + 1, + &Prefix, + &Fcb); + if (!NT_SUCCESS(Irp->IoStatus.Status)) { - Dcb = (PNP_DCB)Ccb; - Irp->IoStatus.Status = NpFindRelativePrefix(Dcb, - &FileName, - 1, - &Prefix, - &Fcb); - if (!NT_SUCCESS(Irp->IoStatus.Status)) goto Quickie; - } - else if ((Type != NPFS_NTC_CCB) || (FileName.Length)) - { - Irp->IoStatus.Status = STATUS_OBJECT_NAME_INVALID; goto Quickie; } - else - { - Prefix.Length = 0; - } + } + else if ((Type != NPFS_NTC_CCB) || (FileName.Length)) + { + Irp->IoStatus.Status = STATUS_OBJECT_NAME_INVALID; + goto Quickie; } else { - if ((FileName.Length <= sizeof(WCHAR)) || - (FileName.Buffer[0] != OBJ_NAME_PATH_SEPARATOR)) - { - Irp->IoStatus.Status = STATUS_OBJECT_NAME_INVALID; - goto Quickie; - } - - Fcb = NpFindPrefix(&FileName, 1, &Prefix); - } - - if (Prefix.Length) - { - Irp->IoStatus.Status = Fcb->NodeType != NPFS_NTC_FCB ? - STATUS_OBJECT_NAME_NOT_FOUND : - STATUS_OBJECT_NAME_INVALID; - } - - if (!Fcb->CurrentInstances) - { - Irp->IoStatus.Status = STATUS_OBJECT_NAME_NOT_FOUND; - } - else - { - Irp->IoStatus = NpCreateClientEnd(Fcb, - FileObject, - DesiredAccess, - IoStack->Parameters.CreatePipe. - SecurityContext->SecurityQos, - IoStack->Parameters.CreatePipe. - SecurityContext->AccessState, - IoStack->Flags & - SL_FORCE_ACCESS_CHECK ? - UserMode : Irp->RequestorMode, - Irp->Tail.Overlay.Thread, - &List); - } - } + Prefix.Length = 0; + } + } + else + { + if ((FileName.Length <= sizeof(OBJ_NAME_PATH_SEPARATOR)) || + (FileName.Buffer[0] != OBJ_NAME_PATH_SEPARATOR)) + { + Irp->IoStatus.Status = STATUS_OBJECT_NAME_INVALID; + goto Quickie; + } + + Fcb = NpFindPrefix(&FileName, TRUE, &Prefix); + } + + if (Prefix.Length) + { + Irp->IoStatus.Status = Fcb->NodeType != NPFS_NTC_FCB ? + STATUS_OBJECT_NAME_NOT_FOUND : + STATUS_OBJECT_NAME_INVALID; + goto Quickie; + } + + if (Fcb->NodeType != NPFS_NTC_FCB) + { + Irp->IoStatus.Status = STATUS_OBJECT_NAME_INVALID; + goto Quickie; + } + + if (!Fcb->ServerOpenCount) + { + Irp->IoStatus.Status = STATUS_OBJECT_NAME_NOT_FOUND; + goto Quickie; + } + + Irp->IoStatus = NpCreateClientEnd(Fcb, + FileObject, + DesiredAccess, + IoStack->Parameters.CreatePipe. + SecurityContext->SecurityQos, + IoStack->Parameters.CreatePipe. + SecurityContext->AccessState, + IoStack->Flags & + SL_FORCE_ACCESS_CHECK ? + UserMode : Irp->RequestorMode, + Irp->Tail.Overlay.Thread, + &List);
Quickie: ExReleaseResourceLite(&NpVcb->Lock); - ASSERT(IsListEmpty(&List) == TRUE); + + NextEntry = List.Flink; + while (NextEntry != &List) + { + ThisEntry = NextEntry; + NextEntry = NextEntry->Flink; + + ListIrp = CONTAINING_RECORD(ThisEntry, IRP, Tail.Overlay.ListEntry); + IoCompleteRequest(ListIrp, IO_DISK_INCREMENT); + } + FsRtlExitFileSystem();
IoCompleteRequest(Irp, IO_NO_INCREMENT); @@ -582,10 +604,11 @@ NpSetFileObject(FileObject, Ccb, Ccb->NonPagedCcb, TRUE); Ccb->FileObject[FILE_PIPE_SERVER_END] = FileObject;
- NpCheckForNotify(Dcb, 1, List); + NpCheckForNotify(Dcb, TRUE, List);
IoStatus->Status = STATUS_SUCCESS; - IoStatus->Information = 2; + IoStatus->Information = FILE_CREATED; + return STATUS_SUCCESS;
Quickie: @@ -605,11 +628,13 @@ USHORT Disposition, ShareAccess; PEPROCESS Process; LIST_ENTRY LocalList; + PLIST_ENTRY NextEntry, ThisEntry; UNICODE_STRING FileName; PNP_FCB Fcb; UNICODE_STRING Prefix; PNAMED_PIPE_CREATE_PARAMETERS Parameters; IO_STATUS_BLOCK IoStatus; + PIRP ListIrp;
DPRINT1("NpFsdCreateNamedPipe(DeviceObject %p Irp %p)\n", DeviceObject, Irp);
@@ -655,16 +680,21 @@ TRUE, &Prefix, &Fcb); - if (!NT_SUCCESS(IoStatus.Status)) goto Quickie; + if (!NT_SUCCESS(IoStatus.Status)) + { + goto Quickie; + } } else { - if (FileName.Length <= 2u || *FileName.Buffer != '\') + if (FileName.Length <= sizeof(OBJ_NAME_PATH_SEPARATOR) || + FileName.Buffer[0] != OBJ_NAME_PATH_SEPARATOR) { IoStatus.Status = STATUS_OBJECT_NAME_INVALID; goto Quickie; } - Fcb = NpFindPrefix(&FileName, 1u, &Prefix); + + Fcb = NpFindPrefix(&FileName, TRUE, &Prefix); }
if (Prefix.Length) @@ -683,7 +713,7 @@ Parameters, Process, &LocalList, - &Irp->IoStatus); + &IoStatus); goto Quickie; } else @@ -692,30 +722,40 @@ goto Quickie; } } + if (Fcb->NodeType != NPFS_NTC_FCB) { IoStatus.Status = STATUS_OBJECT_NAME_INVALID; goto Quickie; }
- Irp->IoStatus = NpCreateExistingNamedPipe(Fcb, - FileObject, - IoStack->Parameters.CreatePipe. - SecurityContext->DesiredAccess, - IoStack->Parameters.CreatePipe. - SecurityContext->AccessState, - IoStack->Flags & - SL_FORCE_ACCESS_CHECK ? - UserMode : Irp->RequestorMode, - Disposition, - ShareAccess, - Parameters, - Process, - &LocalList); + IoStatus = NpCreateExistingNamedPipe(Fcb, + FileObject, + IoStack->Parameters.CreatePipe. + SecurityContext->DesiredAccess, + IoStack->Parameters.CreatePipe. + SecurityContext->AccessState, + IoStack->Flags & + SL_FORCE_ACCESS_CHECK ? + UserMode : Irp->RequestorMode, + Disposition, + ShareAccess, + Parameters, + Process, + &LocalList);
Quickie: ExReleaseResourceLite(&NpVcb->Lock); - ASSERT(IsListEmpty(&LocalList)); + + NextEntry = LocalList.Flink; + while (NextEntry != &LocalList) + { + ThisEntry = NextEntry; + NextEntry = NextEntry->Flink; + + ListIrp = CONTAINING_RECORD(ThisEntry, IRP, Tail.Overlay.ListEntry); + IoCompleteRequest(ListIrp, IO_DISK_INCREMENT); + }
FsRtlExitFileSystem();
Modified: trunk/reactos/drivers/filesystems/npfs_new/datasup.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/datasup.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/datasup.c [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -437,7 +437,7 @@ ASSERT(ByteOffset < DataEntry->DataSize); ASSERT(DataQueue->EntriesInQueue == 1); } - + InsertTailList(&DataQueue->Queue, &DataEntry->QueueEntry);
if (Status == STATUS_PENDING)
Modified: trunk/reactos/drivers/filesystems/npfs_new/fileinfo.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/fileinfo.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/fileinfo.c [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -327,7 +327,7 @@ { InfoBuffer->ReadDataAvailable = InQueue->BytesInQueue - InQueue->ByteOffset; } - InfoBuffer->WriteQuotaAvailable = OutQueue->QuotaUsed; + InfoBuffer->WriteQuotaAvailable = OutQueue->Quota - OutQueue->QuotaUsed; } else { @@ -335,7 +335,7 @@ { InfoBuffer->ReadDataAvailable = OutQueue->BytesInQueue - OutQueue->ByteOffset; } - InfoBuffer->WriteQuotaAvailable = InQueue->QuotaUsed; + InfoBuffer->WriteQuotaAvailable = OutQueue->Quota - InQueue->QuotaUsed; }
return STATUS_SUCCESS;
Modified: trunk/reactos/drivers/filesystems/npfs_new/npfs.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/npfs.h [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/npfs.h [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -181,6 +181,7 @@ LIST_ENTRY NotifyList; LIST_ENTRY NotifyList2; LIST_ENTRY FcbList; + ULONG Pad;
// // Common Footer @@ -212,6 +213,8 @@ // NP_CB_FOOTER; } NP_FCB, *PNP_FCB; + +C_ASSERT(FIELD_OFFSET(NP_FCB, PrefixTableEntry) == FIELD_OFFSET(NP_DCB, PrefixTableEntry));
// // The nonpaged portion of the CCB
Modified: trunk/reactos/drivers/filesystems/npfs_new/statesup.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/statesup.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/statesup.c [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -96,7 +96,7 @@ break;
case FILE_PIPE_CONNECTED_STATE: - + EventBuffer = NonPagedCcb->EventBuffer[FILE_PIPE_CLIENT_END];
while (Ccb->DataQueue[FILE_PIPE_INBOUND].QueueState != Empty) @@ -121,12 +121,13 @@
if (EventBuffer) KeSetEvent(EventBuffer->Event, IO_NO_INCREMENT, FALSE);
- Status = STATUS_SUCCESS; - break; + // drop down on purpose... queue will be empty so flush code is nop + ASSERT(Ccb->DataQueue[FILE_PIPE_OUTBOUND].QueueState == Empty);
case FILE_PIPE_CLOSING_STATE:
EventBuffer = NonPagedCcb->EventBuffer[FILE_PIPE_CLIENT_END]; + while (Ccb->DataQueue[FILE_PIPE_INBOUND].QueueState != Empty) { Irp = NpRemoveDataQueueEntry(&Ccb->DataQueue[FILE_PIPE_INBOUND], FALSE, List); @@ -177,11 +178,11 @@ { case FILE_PIPE_DISCONNECTED_STATE:
- Status = NpCancelWaiter(&NpVcb->WaitQueue, - &Ccb->Fcb->FullName, - STATUS_SUCCESS, - List); - if (!NT_SUCCESS(Status)) return Status; + Status = NpCancelWaiter(&NpVcb->WaitQueue, + &Ccb->Fcb->FullName, + STATUS_SUCCESS, + List); + if (!NT_SUCCESS(Status)) return Status;
// // Drop down on purpose @@ -203,7 +204,7 @@
Ccb->NamedPipeState = FILE_PIPE_LISTENING_STATE; IoMarkIrpPending(Irp); - InsertTailList(List, &Irp->Tail.Overlay.ListEntry); + InsertTailList(&Ccb->IrpList, &Irp->Tail.Overlay.ListEntry); return STATUS_PENDING;
case FILE_PIPE_CONNECTED_STATE:
Modified: trunk/reactos/drivers/filesystems/npfs_new/strucsup.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs_ne... ============================================================================== --- trunk/reactos/drivers/filesystems/npfs_new/strucsup.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs_new/strucsup.c [iso-8859-1] Wed Sep 11 02:04:17 2013 @@ -53,7 +53,7 @@ PAGED_CODE();
Dcb = Fcb->ParentDcb; - if (!Fcb->CurrentInstances) KeBugCheckEx(NPFS_FILE_SYSTEM, 0x17025F, 0, 0, 0); + if (Fcb->CurrentInstances) KeBugCheckEx(NPFS_FILE_SYSTEM, 0x17025F, 0, 0, 0);
NpCancelWaiter(&NpVcb->WaitQueue, &Fcb->FullName, @@ -205,23 +205,26 @@ BOOLEAN RootPipe; PWCHAR NameBuffer; ULONG BufferOffset; - USHORT PipeNameLength; - PAGED_CODE(); - - PipeNameLength = PipeName->Length; - - if ((PipeNameLength < sizeof(WCHAR)) || - ((PipeNameLength + sizeof(WCHAR)) < PipeNameLength) || - (PipeName->Buffer[0] != OBJ_NAME_PATH_SEPARATOR)) + USHORT Length, MaximumLength; + PAGED_CODE(); + + Length = PipeName->Length; + MaximumLength = Length + sizeof(UNICODE_NULL); + + if ((Length < sizeof(WCHAR)) || (MaximumLength < Length)) { return STATUS_INVALID_PARAMETER; }
RootPipe = FALSE; - if (PipeNameLength == sizeof(WCHAR)) - { + if (PipeName->Buffer[0] != OBJ_NAME_PATH_SEPARATOR) + { + MaximumLength += sizeof(OBJ_NAME_PATH_SEPARATOR); RootPipe = TRUE; - PipeNameLength += sizeof(WCHAR); + if (MaximumLength < sizeof(WCHAR)) + { + return STATUS_INVALID_PARAMETER; + } }
Fcb = ExAllocatePoolWithTag(PagedPool, sizeof(*Fcb), NPFS_FCB_TAG); @@ -235,7 +238,7 @@ InitializeListHead(&Fcb->CcbList);
NameBuffer = ExAllocatePoolWithTag(PagedPool, - PipeName->Length + (RootPipe ? 4 : 2), + MaximumLength, NPFS_NAME_BLOCK_TAG); if (!NameBuffer) { @@ -248,19 +251,19 @@ BufferOffset = 0; if (RootPipe) { - *NameBuffer = OBJ_NAME_PATH_SEPARATOR; + NameBuffer[0] = OBJ_NAME_PATH_SEPARATOR; BufferOffset = 1; }
- RtlCopyMemory(NameBuffer + BufferOffset, PipeName->Buffer, PipeNameLength); - NameBuffer[BufferOffset + (PipeNameLength / sizeof(WCHAR))] = UNICODE_NULL; - - Fcb->FullName.Length = PipeNameLength - sizeof(WCHAR); - Fcb->FullName.MaximumLength = PipeNameLength; + RtlCopyMemory(NameBuffer + BufferOffset, PipeName->Buffer, Length); + NameBuffer[BufferOffset + (Length / sizeof(WCHAR))] = UNICODE_NULL; + + Fcb->FullName.Length = Length; + Fcb->FullName.MaximumLength = MaximumLength; Fcb->FullName.Buffer = NameBuffer;
- Fcb->ShortName.MaximumLength = PipeNameLength - sizeof(WCHAR); - Fcb->ShortName.Length = PipeNameLength - 2 * sizeof(WCHAR); + Fcb->ShortName.MaximumLength = Length; + Fcb->ShortName.Length = Length - sizeof(OBJ_NAME_PATH_SEPARATOR); Fcb->ShortName.Buffer = NameBuffer + 1;
if (!RtlInsertUnicodePrefix(&NpVcb->PrefixTable, @@ -333,6 +336,8 @@
InsertTailList(&Fcb->CcbList, &Ccb->CcbEntry);
+ Fcb->CurrentInstances++; + Fcb->ServerOpenCount++; InitializeListHead(&Ccb->IrpList); ExInitializeResourceLite(&Ccb->NonPagedCcb->Lock); *NewCcb = Ccb;
-
ion＠svn.reactos.org