Intravision/reactos
1
0
Fork 0
mirror of https://github.com/reactos/reactos.git synced 2024-07-02 10:45:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

[NPFS-NEW]: Fix pool corruption and crashing bugs in NpPeek, which was using sizeof instead of FIELD_OFFSET.

Browse source 
[NPFS-NEW]: Actually implement NpCancelWaiter instead of making it ASSERT.
[NPFS-NEW]: Critical fixes to NpAddWaiter and NpWaitForNamedPipe to fix logic flaws.
NPFS-NEW now behaves without any visible regressions, and exhibits only 2 failures in the kernel32 pipe winetest -- vs 120+ failures with the current NPFS driver. It has 0 ntdll pipe failures, and 0 kmtest pipe failures.

svn path=/trunk/; revision=60070
This commit is contained in:
Alex Ionescu 2013-09-13 07:49:42 +00:00
parent f9cb414f74
commit f2ca6ff5d0
6 changed files with 179 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
reactos/drivers/filesystems/npfs_new/create.c
View file

@ -551,7 +551,7 @@ NpCreateNewNamedPipe(IN PNP_DCB Dcb,
Parameters->NamedPipeType & 0xFFFF,
&Fcb);
if (!NT_SUCCESS(Status)) goto Quickie;
Status = NpCreateCcb(Fcb,
FileObject,
FILE_PIPE_LISTENING_STATE,

37
reactos/drivers/filesystems/npfs_new/fsctrl.c
View file

@ -206,7 +206,7 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
{
PIO_STACK_LOCATION IoStack;
NODE_TYPE_CODE Type;
ULONG InputLength;
ULONG OutputLength;
ULONG NamedPipeEnd;
PNP_CCB Ccb;
PFILE_PIPE_PEEK_BUFFER PeekBuffer;
@ -218,7 +218,7 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
PAGED_CODE();
IoStack = IoGetCurrentIrpStackLocation(Irp);
InputLength = IoStack->Parameters.FileSystemControl.OutputBufferLength;
OutputLength = IoStack->Parameters.FileSystemControl.OutputBufferLength;
Type = NpDecodeFileObject(IoStack->FileObject, NULL, &Ccb, &NamedPipeEnd);
if (!Type)
@ -226,7 +226,8 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
return STATUS_PIPE_DISCONNECTED;
}
if ((Type != NPFS_NTC_CCB) && (InputLength < sizeof(*PeekBuffer)))
if ((Type != NPFS_NTC_CCB) &&
(OutputLength < FIELD_OFFSET(FILE_PIPE_PEEK_BUFFER, Data)))
{
return STATUS_INVALID_PARAMETER;
}
@ -264,7 +265,7 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
PeekBuffer->NumberOfMessages = 0;
PeekBuffer->MessageLength = 0;
PeekBuffer->NamedPipeState = Ccb->NamedPipeState;
BytesPeeked = sizeof(*PeekBuffer);
BytesPeeked = FIELD_OFFSET(FILE_PIPE_PEEK_BUFFER, Data);
if (DataQueue->QueueState == WriteEntries)
{
@ -279,7 +280,8 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
PeekBuffer->NumberOfMessages = DataQueue->EntriesInQueue;
PeekBuffer->MessageLength = DataEntry->DataSize - DataQueue->ByteOffset;
}
if (InputLength == sizeof(*PeekBuffer))
if (OutputLength == FIELD_OFFSET(FILE_PIPE_PEEK_BUFFER, Data))
{
Status = PeekBuffer->ReadDataAvailable ? STATUS_BUFFER_OVERFLOW : STATUS_SUCCESS;
}
@ -289,12 +291,12 @@ NpPeek(IN PDEVICE_OBJECT DeviceObject,
TRUE,
FALSE,
PeekBuffer->Data,
InputLength - sizeof(*PeekBuffer),
OutputLength - FIELD_OFFSET(FILE_PIPE_PEEK_BUFFER, Data),
Ccb->Fcb->NamedPipeType == FILE_PIPE_MESSAGE_TYPE,
Ccb,
List);
Status = IoStatus.Status;
BytesPeeked = IoStatus.Information + sizeof(*PeekBuffer);
BytesPeeked += IoStatus.Information;
}
}
else
@ -523,14 +525,18 @@ NpWaitForNamedPipe(IN PDEVICE_OBJECT DeviceObject,
NODE_TYPE_CODE NodeTypeCode;
PLIST_ENTRY NextEntry;
PNP_FCB Fcb;
PWCHAR OriginalBuffer;
PAGED_CODE();
IoStack = IoGetCurrentIrpStackLocation(Irp);
InLength = IoStack->Parameters.DeviceIoControl.InputBufferLength;
InLength = IoStack->Parameters.FileSystemControl.InputBufferLength;
SourceString.Buffer = NULL;
if (NpDecodeFileObject(IoStack->FileObject, NULL, &Ccb, &NamedPipeEnd) != NPFS_NTC_ROOT_DCB)
if (NpDecodeFileObject(IoStack->FileObject,
NULL,
&Ccb,
&NamedPipeEnd) != NPFS_NTC_ROOT_DCB)
{
Status = STATUS_ILLEGAL_FUNCTION;
goto Quickie;
@ -544,14 +550,17 @@ NpWaitForNamedPipe(IN PDEVICE_OBJECT DeviceObject,
}
NameLength = Buffer->NameLength;
if ((NameLength > 0xFFFD) || ((NameLength + sizeof(*Buffer)) > InLength))
if ((NameLength > (0xFFFF - sizeof(UNICODE_NULL))) ||
((NameLength + FIELD_OFFSET(FILE_PIPE_WAIT_FOR_BUFFER, Name)) > InLength))
{
Status = STATUS_INVALID_PARAMETER;
goto Quickie;
}
SourceString.Length = (USHORT)NameLength + sizeof(OBJ_NAME_PATH_SEPARATOR);
SourceString.Buffer = ExAllocatePoolWithTag(PagedPool, SourceString.Length, NPFS_WRITE_BLOCK_TAG);
SourceString.Buffer = ExAllocatePoolWithTag(PagedPool,
SourceString.Length,
NPFS_WRITE_BLOCK_TAG);
if (!SourceString.Buffer)
{
Status = STATUS_INSUFFICIENT_RESOURCES;
@ -562,6 +571,7 @@ NpWaitForNamedPipe(IN PDEVICE_OBJECT DeviceObject,
RtlCopyMemory(&SourceString.Buffer[1], Buffer->Name, Buffer->NameLength);
Status = STATUS_SUCCESS;
OriginalBuffer = SourceString.Buffer;
//Status = NpTranslateAlias(&SourceString);
if (!NT_SUCCESS(Status)) goto Quickie;
@ -583,7 +593,7 @@ NpWaitForNamedPipe(IN PDEVICE_OBJECT DeviceObject,
if (Ccb->NamedPipeState == FILE_PIPE_LISTENING_STATE) break;
}
if (NextEntry == &Fcb->CcbList)
if (NextEntry != &Fcb->CcbList)
{
Status = STATUS_SUCCESS;
}
@ -592,7 +602,8 @@ NpWaitForNamedPipe(IN PDEVICE_OBJECT DeviceObject,
Status = NpAddWaiter(&NpVcb->WaitQueue,
Fcb->Timeout,
Irp,
&SourceString);
OriginalBuffer == SourceString.Buffer ?
NULL : &SourceString);
}
Quickie:

2
reactos/drivers/filesystems/npfs_new/main.c
View file

@ -62,7 +62,7 @@ DriverEntry(IN PDRIVER_OBJECT DriverObject,
DriverObject->DriverUnload = NULL;
RtlInitUnicodeString(&DeviceName, L"\\Device\\NamedPipe2");
RtlInitUnicodeString(&DeviceName, L"\\Device\\NamedPipe");
Status = IoCreateDevice(DriverObject,
sizeof(NP_VCB),
&DeviceName,

4
reactos/drivers/filesystems/npfs_new/npfs.h
View file

@ -178,7 +178,7 @@ typedef struct _NP_WAIT_QUEUE_ENTRY
KDPC Dpc;
KTIMER Timer;
PNP_WAIT_QUEUE WaitQueue;
UNICODE_STRING String;
UNICODE_STRING AliasName;
PFILE_OBJECT FileObject;
} NP_WAIT_QUEUE_ENTRY, *PNP_WAIT_QUEUE_ENTRY;
@ -590,7 +590,7 @@ NTAPI
NpAddWaiter(IN PNP_WAIT_QUEUE WaitQueue,
IN LARGE_INTEGER WaitTime,
IN PIRP Irp,
IN PUNICODE_STRING Name);
IN PUNICODE_STRING AliasName);
NTSTATUS
NTAPI

3
reactos/drivers/filesystems/npfs_new/strucsup.c
View file

@ -66,7 +66,6 @@ NpDeleteFcb(IN PNP_FCB Fcb,
IN PLIST_ENTRY ListEntry)
{
PNP_DCB Dcb;
PAGED_CODE();
Dcb = Fcb->ParentDcb;
@ -76,7 +75,7 @@ NpDeleteFcb(IN PNP_FCB Fcb,
&Fcb->FullName,
STATUS_OBJECT_NAME_NOT_FOUND,
ListEntry);
RemoveEntryList(&Fcb->DcbEntry);
if (Fcb->SecurityDescriptor)

215
reactos/drivers/filesystems/npfs_new/waitsup.c
View file

@ -45,7 +45,7 @@ NpCancelWaitQueueIrp(IN PDEVICE_OBJECT DeviceObject,
if (WaitEntry)
{
ObfDereferenceObject(WaitEntry->FileObject);
ObDereferenceObject(WaitEntry->FileObject);
ExFreePool(WaitEntry);
}
@ -66,23 +66,28 @@ NpTimerDispatch(IN PKDPC Dpc,
PNP_WAIT_QUEUE_ENTRY WaitEntry = Context;
OldIrql = KfAcquireSpinLock(&WaitEntry->WaitQueue->WaitLock);
Irp = WaitEntry->Irp;
if (Irp)
{
RemoveEntryList(&Irp->Tail.Overlay.ListEntry);
if (!IoSetCancelRoutine(Irp, NULL))
{
Irp->Tail.Overlay.DriverContext[1] = NULL;
Irp = NULL;
}
}
KfReleaseSpinLock(&WaitEntry->WaitQueue->WaitLock, OldIrql);
if (Irp)
{
Irp->IoStatus.Status = STATUS_IO_TIMEOUT;
IoCompleteRequest(Irp, IO_NAMED_PIPE_INCREMENT);
}
ObfDereferenceObject(WaitEntry->FileObject);
ObDereferenceObject(WaitEntry->FileObject);
ExFreePool(WaitEntry);
}
@ -99,13 +104,22 @@ NTAPI
NpCancelWaiter(IN PNP_WAIT_QUEUE WaitQueue,
IN PUNICODE_STRING PipeName,
IN NTSTATUS Status,
IN PLIST_ENTRY ListEntry)
IN PLIST_ENTRY List)
{
UNICODE_STRING DestinationString;
KIRQL OldIrql;
PWCHAR Buffer;
PLIST_ENTRY NextEntry;
PNP_WAIT_QUEUE_ENTRY WaitEntry, Linkage;
PIRP WaitIrp;
PFILE_PIPE_WAIT_FOR_BUFFER WaitBuffer;
ULONG i, NameLength;
Buffer = ExAllocatePoolWithTag(NonPagedPool, PipeName->Length, NPFS_WAIT_BLOCK_TAG);
Linkage = NULL;
Buffer = ExAllocatePoolWithTag(NonPagedPool,
PipeName->Length,
NPFS_WAIT_BLOCK_TAG);
if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES;
RtlInitEmptyUnicodeString(&DestinationString, Buffer, PipeName->Length);
@ -113,10 +127,78 @@ NpCancelWaiter(IN PNP_WAIT_QUEUE WaitQueue,
OldIrql = KfAcquireSpinLock(&WaitQueue->WaitLock);
ASSERT(IsListEmpty(&WaitQueue->WaitList) == TRUE);
for (NextEntry = WaitQueue->WaitList.Flink;
NextEntry != &WaitQueue->WaitList;
NextEntry = NextEntry->Flink)
{
WaitIrp = CONTAINING_RECORD(NextEntry, IRP, Tail.Overlay.ListEntry);
WaitEntry = WaitIrp->Tail.Overlay.DriverContext[1];
if (WaitEntry->AliasName.Length)
{
ASSERT(FALSE);
if (DestinationString.Length == WaitEntry->AliasName.Length)
{
if (RtlCompareMemory(WaitEntry->AliasName.Buffer,
DestinationString.Buffer,
DestinationString.Length) ==
DestinationString.Length)
{
CancelWait:
RemoveEntryList(&WaitIrp->Tail.Overlay.ListEntry);
if (KeCancelTimer(&WaitEntry->Timer))
{
WaitEntry->WaitQueue = (PNP_WAIT_QUEUE)Linkage;
Linkage = WaitEntry;
}
else
{
WaitEntry->Irp = NULL;
WaitIrp->Tail.Overlay.DriverContext[1] = NULL;
}
if (IoSetCancelRoutine(WaitIrp, NULL))
{
WaitIrp->IoStatus.Information = 0;
WaitIrp->IoStatus.Status = Status;
InsertTailList(List, &WaitIrp->Tail.Overlay.ListEntry);
}
else
{
WaitIrp->Tail.Overlay.DriverContext[1] = NULL;
}
}
}
}
else
{
WaitBuffer = WaitIrp->AssociatedIrp.SystemBuffer;
if (WaitBuffer->NameLength + sizeof(WCHAR) == DestinationString.Length)
{
NameLength = WaitBuffer->NameLength / sizeof(WCHAR);
for (i = 0; i < NameLength; i++)
{
if (WaitBuffer->Name[i] != DestinationString.Buffer[i + 1]) break;
}
if (i >= NameLength) goto CancelWait;
}
}
}
KfReleaseSpinLock(&WaitQueue->WaitLock, OldIrql);
ExFreePool(DestinationString.Buffer);
while (Linkage)
{
WaitEntry = Linkage;
Linkage = (PNP_WAIT_QUEUE_ENTRY)Linkage->WaitQueue;
ObDereferenceObject(WaitEntry->FileObject);
ExFreePool(WaitEntry);
}
return STATUS_SUCCESS;
}
@ -124,8 +206,8 @@ NTSTATUS
NTAPI
NpAddWaiter(IN PNP_WAIT_QUEUE WaitQueue,
IN LARGE_INTEGER WaitTime,
IN PIRP Irp,
IN PUNICODE_STRING Name)
IN PIRP Irp,
IN PUNICODE_STRING AliasName)
{
PIO_STACK_LOCATION IoStack;
KIRQL OldIrql;
@ -137,71 +219,74 @@ NpAddWaiter(IN PNP_WAIT_QUEUE WaitQueue,
IoStack = IoGetCurrentIrpStackLocation(Irp);
WaitEntry = ExAllocatePoolWithQuotaTag(NonPagedPool, sizeof(*WaitEntry), NPFS_WRITE_BLOCK_TAG);
if (WaitEntry)
WaitEntry = ExAllocatePoolWithQuotaTag(NonPagedPool,
sizeof(*WaitEntry),
NPFS_WRITE_BLOCK_TAG);
if (!WaitEntry)
{
KeInitializeDpc(&WaitEntry->Dpc, NpTimerDispatch, WaitEntry);
KeInitializeTimer(&WaitEntry->Timer);
return STATUS_INSUFFICIENT_RESOURCES;
}
if (Name)
{
WaitEntry->String = *Name;
}
else
{
WaitEntry->String.Length = 0;
WaitEntry->String.Buffer = 0;
}
KeInitializeDpc(&WaitEntry->Dpc, NpTimerDispatch, WaitEntry);
KeInitializeTimer(&WaitEntry->Timer);
WaitEntry->WaitQueue = WaitQueue;
WaitEntry->Irp = Irp;
WaitBuffer = (PFILE_PIPE_WAIT_FOR_BUFFER)Irp->AssociatedIrp.SystemBuffer;
if (WaitBuffer->TimeoutSpecified)
{
DueTime = WaitBuffer->Timeout;
}
else
{
DueTime = WaitTime;
}
for (i = 0; i < WaitBuffer->NameLength / sizeof(WCHAR); i++)
{
WaitBuffer->Name[i] = RtlUpcaseUnicodeChar(WaitBuffer->Name[i]);
}
Irp->Tail.Overlay.DriverContext[0] = WaitQueue;
Irp->Tail.Overlay.DriverContext[1] = WaitEntry;
OldIrql = KfAcquireSpinLock(&WaitQueue->WaitLock);
IoSetCancelRoutine(Irp, NpCancelWaitQueueIrp);
if (Irp->Cancel && IoSetCancelRoutine(Irp, NULL))
{
Status = STATUS_CANCELLED;
}
else
{
InsertTailList(&WaitQueue->WaitList, &Irp->Tail.Overlay.ListEntry);
IoMarkIrpPending(Irp);
Status = STATUS_PENDING;
WaitEntry->FileObject = IoStack->FileObject;
ObfReferenceObject(WaitEntry->FileObject);
KeSetTimer(&WaitEntry->Timer, DueTime, &WaitEntry->Dpc);
WaitEntry = NULL;
}
KfReleaseSpinLock(&WaitQueue->WaitLock, OldIrql);
if (WaitEntry) ExFreePool(WaitEntry);
if (AliasName)
{
WaitEntry->AliasName = *AliasName;
}
else
{
Status = STATUS_INSUFFICIENT_RESOURCES;
WaitEntry->AliasName.Length = 0;
WaitEntry->AliasName.Buffer = NULL;
}
WaitEntry->WaitQueue = WaitQueue;
WaitEntry->Irp = Irp;
WaitBuffer = (PFILE_PIPE_WAIT_FOR_BUFFER)Irp->AssociatedIrp.SystemBuffer;
if (WaitBuffer->TimeoutSpecified)
{
DueTime = WaitBuffer->Timeout;
}
else
{
DueTime = WaitTime;
}
for (i = 0; i < WaitBuffer->NameLength / sizeof(WCHAR); i++)
{
WaitBuffer->Name[i] = RtlUpcaseUnicodeChar(WaitBuffer->Name[i]);
}
Irp->Tail.Overlay.DriverContext[0] = WaitQueue;
Irp->Tail.Overlay.DriverContext[1] = WaitEntry;
OldIrql = KfAcquireSpinLock(&WaitQueue->WaitLock);
IoSetCancelRoutine(Irp, NpCancelWaitQueueIrp);
if (Irp->Cancel && IoSetCancelRoutine(Irp, NULL))
{
Status = STATUS_CANCELLED;
}
else
{
InsertTailList(&WaitQueue->WaitList, &Irp->Tail.Overlay.ListEntry);
IoMarkIrpPending(Irp);
Status = STATUS_PENDING;
WaitEntry->FileObject = IoStack->FileObject;
ObReferenceObject(WaitEntry->FileObject);
KeSetTimer(&WaitEntry->Timer, DueTime, &WaitEntry->Dpc);
WaitEntry = NULL;
}
KfReleaseSpinLock(&WaitQueue->WaitLock, OldIrql);
if (WaitEntry) ExFreePool(WaitEntry);
return Status;
}