From ca2a89a4577ddb1937e3f7aa623519049e8a05a1 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Sat, 29 Jul 2006 16:56:26 +0000 Subject: [PATCH] - Add another paramter to IopCleanupFailedIrp to free an optional buffer being specified to it. This way we don't leak some allocated buffers when IRP allocation fails. - Create inlined IopUnQueueIrpFromThread to match IopQueueIrpToThread. svn path=/trunk/; revision=23352 --- reactos/ntoskrnl/KrnlFun.c | 1 - reactos/ntoskrnl/include/internal/io.h | 3 ++- reactos/ntoskrnl/include/internal/io_x.h | 9 ++++++++ reactos/ntoskrnl/io/iomgr/file.c | 5 ++--- reactos/ntoskrnl/io/iomgr/iofunc.c | 28 ++++++++++++------------ reactos/ntoskrnl/io/iomgr/irp.c | 12 +++++----- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/reactos/ntoskrnl/KrnlFun.c b/reactos/ntoskrnl/KrnlFun.c index 621315cafa8..a9aa4abc50d 100644 --- a/reactos/ntoskrnl/KrnlFun.c +++ b/reactos/ntoskrnl/KrnlFun.c @@ -10,7 +10,6 @@ // // Io: // - See why queueing IRPs and cancelling them causes crashes. -// - Add another parameter to IopCleanupFailedIrp. // - Add Access Checks in IopParseDevice. // - Add validation checks in IoCreateFile. // - Add probe/alignment checks for Query/Set routines. diff --git a/reactos/ntoskrnl/include/internal/io.h b/reactos/ntoskrnl/include/internal/io.h index 6d0867ca4db..1e30e5e883f 100644 --- a/reactos/ntoskrnl/include/internal/io.h +++ b/reactos/ntoskrnl/include/internal/io.h @@ -666,7 +666,8 @@ NTSTATUS NTAPI IopCleanupFailedIrp( IN PFILE_OBJECT FileObject, - IN PKEVENT EventObject + IN PKEVENT EventObject, + IN PVOID Buffer OPTIONAL ); VOID diff --git a/reactos/ntoskrnl/include/internal/io_x.h b/reactos/ntoskrnl/include/internal/io_x.h index 78901d264d2..068b6bc1309 100644 --- a/reactos/ntoskrnl/include/internal/io_x.h +++ b/reactos/ntoskrnl/include/internal/io_x.h @@ -43,6 +43,15 @@ IopQueueIrpToThread(IN PIRP Irp) KeLowerIrql(OldIrql); } +VOID +FORCEINLINE +IopUnQueueIrpFromThread(IN PIRP Irp) +{ + /* Remove it from the list and reset it */ + RemoveEntryList(&Irp->ThreadListEntry); + InitializeListHead(&Irp->ThreadListEntry); +} + VOID static __inline IopUpdateOperationCount(IN IOP_TRANSFER_TYPE Type) diff --git a/reactos/ntoskrnl/io/iomgr/file.c b/reactos/ntoskrnl/io/iomgr/file.c index 35dcb73b63a..e7086eda399 100644 --- a/reactos/ntoskrnl/io/iomgr/file.c +++ b/reactos/ntoskrnl/io/iomgr/file.c @@ -404,8 +404,7 @@ IopParseDevice(IN PVOID ParseObject, FileObject->Event.Header.SignalState = 1; /* Now that we've signaled the events, de-associate the IRP */ - //RemoveEntryList(&Irp->ThreadListEntry); - //InitializeListHead(&Irp->ThreadListEntry); + //IopUnQueueIrpFromThread(Irp); /* Check if the IRP had an input buffer */ if ((Irp->Flags & IRP_BUFFERED_IO) && @@ -795,7 +794,7 @@ IopSecurityFile(IN PVOID ObjectBody, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, NULL); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; diff --git a/reactos/ntoskrnl/io/iomgr/iofunc.c b/reactos/ntoskrnl/io/iomgr/iofunc.c index b54487abb1e..110b6c07626 100644 --- a/reactos/ntoskrnl/io/iomgr/iofunc.c +++ b/reactos/ntoskrnl/io/iomgr/iofunc.c @@ -346,7 +346,7 @@ IopDeviceFsIoControl(IN HANDLE DeviceHandle, FALSE, EventObject, IoStatusBlock); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, Event, NULL); /* Set some extra settings */ Irp->Tail.Overlay.AuxiliaryBuffer = (PVOID) NULL; @@ -415,7 +415,7 @@ IopQueryDeviceInformation(IN PFILE_OBJECT FileObject, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, NULL); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -673,7 +673,7 @@ IoSetInformation(IN PFILE_OBJECT FileObject, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, NULL); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -887,7 +887,7 @@ NtFlushBuffersFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, TRUE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set up the IRP */ Irp->Flags = (LocalEvent) ? IRP_SYNCHRONOUS_API : 0; @@ -1017,7 +1017,7 @@ NtNotifyChangeDirectoryFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, Event, NULL); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; @@ -1165,7 +1165,7 @@ NtLockFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, Event, NULL); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; @@ -1366,7 +1366,7 @@ NtQueryDirectoryFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, TRUE); - if (!Irp) return IopCleanupFailedIrp(FileObject, EventHandle); + if (!Irp) return IopCleanupFailedIrp(FileObject, EventHandle, AuxBuffer); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; @@ -1587,7 +1587,7 @@ NtQueryInformationFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -1882,7 +1882,7 @@ NtReadFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, NULL); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -2129,7 +2129,7 @@ NtSetInformationFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, TRUE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -2463,7 +2463,7 @@ NtUnlockFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; @@ -2693,7 +2693,7 @@ NtWriteFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, NULL); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, NULL); /* Set the IRP */ Irp->Tail.Overlay.OriginalFileObject = FileObject; @@ -2885,7 +2885,7 @@ NtQueryVolumeInformationFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; @@ -3035,7 +3035,7 @@ NtSetVolumeInformationFile(IN HANDLE FileHandle, /* Allocate the IRP */ Irp = IoAllocateIrp(DeviceObject->StackSize, FALSE); - if (!Irp) return IopCleanupFailedIrp(FileObject, Event); + if (!Irp) return IopCleanupFailedIrp(FileObject, NULL, Event); /* Set up the IRP */ Irp->RequestorMode = PreviousMode; diff --git a/reactos/ntoskrnl/io/iomgr/irp.c b/reactos/ntoskrnl/io/iomgr/irp.c index 1eae7838a4b..21955c67a43 100644 --- a/reactos/ntoskrnl/io/iomgr/irp.c +++ b/reactos/ntoskrnl/io/iomgr/irp.c @@ -43,13 +43,17 @@ IopAbortIrpKernelApc(IN PKAPC Apc) NTSTATUS NTAPI IopCleanupFailedIrp(IN PFILE_OBJECT FileObject, - IN PKEVENT EventObject) + IN PKEVENT EventObject OPTIONAL, + IN PVOID Buffer OPTIONAL) { PAGED_CODE(); /* Dereference the event */ if (EventObject) ObDereferenceObject(EventObject); + /* Free a buffer, if any */ + if (Buffer) ExFreePool(Buffer); + /* If this was a file opened for synch I/O, then unlock it */ if (FileObject->Flags & FO_SYNCHRONOUS_IO) IopUnlockFileObject(FileObject); @@ -348,8 +352,7 @@ IopCompleteRequest(IN PKAPC Apc, } /* Now that we've signaled the events, de-associate the IRP */ - RemoveEntryList(&Irp->ThreadListEntry); - InitializeListHead(&Irp->ThreadListEntry); + IopUnQueueIrpFromThread(Irp); /* Now check if a User APC Routine was requested */ if (Irp->Overlay.AsynchronousParameters.UserApcRoutine) @@ -447,8 +450,7 @@ IopCompleteRequest(IN PKAPC Apc, } /* Now that we've signaled the events, de-associate the IRP */ - RemoveEntryList(&Irp->ThreadListEntry); - InitializeListHead(&Irp->ThreadListEntry); + IopUnQueueIrpFromThread(Irp); /* Free the IRP as well */ IoFreeIrp(Irp);