[cgutman] 42387: - Finally get IRP_MJ_CLOSE working properly - Remove handling of IRP_MJ_CLEANUP and move the code to the DispatchClose routine - Remove the hack (holding an extra reference and not closing the handle) which hid these bugs - Fixes some memory and handle leaks too
Author: cgutman Date: Tue Aug 4 20:56:56 2009 New Revision: 42387
URL: http://svn.reactos.org/svn/reactos?rev=42387&view=rev Log: - Finally get IRP_MJ_CLOSE working properly - Remove handling of IRP_MJ_CLEANUP and move the code to the DispatchClose routine - Remove the hack (holding an extra reference and not closing the handle) which hid these bugs - Fixes some memory and handle leaks too
Modified: trunk/reactos/drivers/network/afd/afd/bind.c trunk/reactos/drivers/network/afd/afd/connect.c trunk/reactos/drivers/network/afd/afd/main.c trunk/reactos/drivers/network/afd/afd/tdi.c trunk/reactos/drivers/network/tcpip/include/fileobjs.h trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c trunk/reactos/drivers/network/tcpip/tcpip/main.c
Modified: trunk/reactos/drivers/network/afd/afd/bind.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/bin... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/bind.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/bind.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -32,11 +32,6 @@ FCB->LocalAddress, &FCB->AddressFile.Handle, &FCB->AddressFile.Object ); - - if (NT_SUCCESS(Status)) - { - ObReferenceObject(FCB->AddressFile.Object); - }
AFD_DbgPrint(MID_TRACE,("Returning %x\n", Status));
Modified: trunk/reactos/drivers/network/afd/afd/connect.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/con... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/connect.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/connect.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -29,11 +29,6 @@ FCB->Connection.Object ); }
- if (NT_SUCCESS(Status)) - { - ObReferenceObject(FCB->Connection.Object); - } - return Status; }
Modified: trunk/reactos/drivers/network/afd/afd/main.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/mai... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/main.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/main.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -96,6 +96,8 @@ FCB->DeviceExt = DeviceExt; FCB->Recv.Size = DEFAULT_RECEIVE_WINDOW_SIZE; FCB->Send.Size = DEFAULT_SEND_WINDOW_SIZE; + FCB->AddressFile.Handle = INVALID_HANDLE_VALUE; + FCB->Connection.Handle = INVALID_HANDLE_VALUE;
KeInitializeSpinLock( &FCB->SpinLock ); ExInitializeFastMutex( &FCB->Mutex ); @@ -167,13 +169,19 @@ return Status; }
-VOID CleanupSocket( PAFD_FCB FCB ) { +static NTSTATUS NTAPI +AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp, + PIO_STACK_LOCATION IrpSp) +{ + PFILE_OBJECT FileObject = IrpSp->FileObject; + PAFD_FCB FCB = FileObject->FsContext; UINT i; PAFD_IN_FLIGHT_REQUEST InFlightRequest[IN_FLIGHT_REQUESTS];
- AFD_DbgPrint(MIN_TRACE,("Called (%x)\n", FCB)); - - if( !SocketAcquireStateLock( FCB ) ) return; + AFD_DbgPrint(MID_TRACE, + ("AfdClose(DeviceObject %p Irp %p)\n", DeviceObject, Irp)); + + if( !SocketAcquireStateLock( FCB ) ) return STATUS_FILE_CLOSED;
FCB->State = SOCKET_STATE_CLOSED;
@@ -191,104 +199,53 @@ } }
- FCB->State = SOCKET_STATE_CREATED; - - if( FCB->EventSelect ) { + KillSelectsForFCB( FCB->DeviceExt, FileObject, FALSE ); + + SocketStateUnlock( FCB ); + + if( FCB->EventSelect ) ObDereferenceObject( FCB->EventSelect ); - FCB->EventSelect = NULL; - } - if( FCB->Context ) { + + if( FCB->Context ) ExFreePool( FCB->Context ); - FCB->Context = NULL; - } - if( FCB->Recv.Window ) { + + if( FCB->Recv.Window ) ExFreePool( FCB->Recv.Window ); - FCB->Recv.Window = NULL; - } - if( FCB->Send.Window ) { + + if( FCB->Send.Window ) ExFreePool( FCB->Send.Window ); - FCB->Send.Window = NULL; - } - if( FCB->AddressFrom ) { + + if( FCB->AddressFrom ) ExFreePool( FCB->AddressFrom ); - FCB->AddressFrom = NULL; - } - if( FCB->LocalAddress ) { + + if( FCB->LocalAddress ) ExFreePool( FCB->LocalAddress ); - FCB->LocalAddress = NULL; - } - if( FCB->RemoteAddress ) { + + if( FCB->RemoteAddress ) ExFreePool( FCB->RemoteAddress ); - FCB->RemoteAddress = NULL; - } - if( FCB->Connection.Object ) { - ZwClose(FCB->Connection.Handle); + + if( FCB->Connection.Object ) ObDereferenceObject(FCB->Connection.Object); - FCB->Connection.Object = NULL; - } - if( FCB->AddressFile.Object ) { - ZwClose(FCB->AddressFile.Handle); + + if( FCB->AddressFile.Object ) ObDereferenceObject(FCB->AddressFile.Object); - FCB->AddressFile.Object = NULL; - } - - SocketStateUnlock( FCB ); -} - -VOID DestroySocket( PAFD_FCB FCB ) { + + if( FCB->AddressFile.Handle != INVALID_HANDLE_VALUE ) + ZwClose(FCB->AddressFile.Handle); + + if( FCB->Connection.Handle != INVALID_HANDLE_VALUE ) + ZwClose(FCB->Connection.Handle); + if( FCB->TdiDeviceName.Buffer ) ExFreePool(FCB->TdiDeviceName.Buffer);
ExFreePool(FCB); - AFD_DbgPrint(MIN_TRACE,("Deleted (%x)\n", FCB)); -} - -static NTSTATUS NTAPI -AfdCloseSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp, - PIO_STACK_LOCATION IrpSp) -{ - PFILE_OBJECT FileObject = IrpSp->FileObject; - PAFD_FCB FCB = FileObject->FsContext; - - AFD_DbgPrint(MID_TRACE, - ("AfdClose(DeviceObject %p Irp %p)\n", DeviceObject, Irp)); - - if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp ); - - AFD_DbgPrint(MID_TRACE,("FCB %x\n", FCB)); - - FileObject->FsContext = NULL; - SocketStateUnlock( FCB ); - - DestroySocket( FCB );
Irp->IoStatus.Status = STATUS_SUCCESS; Irp->IoStatus.Information = 0; IoCompleteRequest(Irp, IO_NETWORK_INCREMENT);
AFD_DbgPrint(MID_TRACE, ("Returning success.\n")); - - return STATUS_SUCCESS; -} - -static NTSTATUS NTAPI -AfdCleanupSocket(PDEVICE_OBJECT DeviceObject, PIRP Irp, - PIO_STACK_LOCATION IrpSp) -{ - PFILE_OBJECT FileObject = IrpSp->FileObject; - PAFD_FCB FCB = FileObject->FsContext; - - if( !SocketAcquireStateLock( FCB ) ) return LostSocket( Irp ); - - CleanupSocket( FCB ); - - KillSelectsForFCB( FCB->DeviceExt, FileObject, FALSE ); - - SocketStateUnlock( FCB ); - - Irp->IoStatus.Status = STATUS_SUCCESS; - Irp->IoStatus.Information = 0; - IoCompleteRequest(Irp, IO_NETWORK_INCREMENT);
return STATUS_SUCCESS; } @@ -373,9 +330,6 @@ /* Ditto the borrowing */ return AfdCloseSocket(DeviceObject, Irp, IrpSp);
- case IRP_MJ_CLEANUP: - return AfdCleanupSocket(DeviceObject, Irp, IrpSp); - /* write data */ case IRP_MJ_WRITE: return AfdConnectedSocketWriteData( DeviceObject, Irp, IrpSp, TRUE ); @@ -550,7 +504,6 @@ /* register driver routines */ DriverObject->MajorFunction[IRP_MJ_CLOSE] = AfdDispatch; DriverObject->MajorFunction[IRP_MJ_CREATE] = AfdDispatch; - DriverObject->MajorFunction[IRP_MJ_CLEANUP] = AfdDispatch; DriverObject->MajorFunction[IRP_MJ_WRITE] = AfdDispatch; DriverObject->MajorFunction[IRP_MJ_READ] = AfdDispatch; DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = AfdDispatch;
Modified: trunk/reactos/drivers/network/afd/afd/tdi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/afd/afd/tdi... ============================================================================== --- trunk/reactos/drivers/network/afd/afd/tdi.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/afd/afd/tdi.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -146,13 +146,12 @@ }
if (!NT_SUCCESS(Status)) { - *Handle = NULL; + *Handle = INVALID_HANDLE_VALUE; *Object = NULL; }
return Status; } -
NTSTATUS TdiOpenAddressFile( PUNICODE_STRING DeviceName,
Modified: trunk/reactos/drivers/network/tcpip/include/fileobjs.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/inclu... ============================================================================== --- trunk/reactos/drivers/network/tcpip/include/fileobjs.h [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/include/fileobjs.h [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -23,9 +23,6 @@ NTSTATUS FileCloseAddress( PTDI_REQUEST Request);
-NTSTATUS FileFreeAddress( - PTDI_REQUEST Request); - NTSTATUS FileOpenConnection( PTDI_REQUEST Request, PVOID ClientContext); @@ -35,13 +32,10 @@ NTSTATUS FileCloseConnection( PTDI_REQUEST Request);
-NTSTATUS FileFreeConnection( - PTDI_REQUEST Request); - NTSTATUS FileOpenControlChannel( PTDI_REQUEST Request);
-NTSTATUS FileFreeControlChannel( +NTSTATUS FileCloseControlChannel( PTDI_REQUEST Request);
#endif /* __FILEOBJS_H */
Modified: trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip... ============================================================================== --- trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -284,17 +284,21 @@ NTSTATUS FileCloseAddress( PTDI_REQUEST Request) { - KIRQL OldIrql; PADDRESS_FILE AddrFile; NTSTATUS Status = STATUS_SUCCESS; + KIRQL OldIrql; + PDATAGRAM_RECEIVE_REQUEST ReceiveRequest; PDATAGRAM_SEND_REQUEST SendRequest; - PDATAGRAM_RECEIVE_REQUEST ReceiveRequest; - PLIST_ENTRY CurrentEntry; - PLIST_ENTRY NextEntry; + PLIST_ENTRY CurrentEntry, NextEntry; + + AddrFile = Request->Handle.AddressHandle;
TI_DbgPrint(MID_TRACE, ("Called.\n"));
- AddrFile = Request->Handle.AddressHandle; + /* Remove address file from the global list */ + TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql); + RemoveEntryList(&AddrFile->ListEntry); + TcpipReleaseSpinLock(&AddressFileListLock, OldIrql);
TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql);
@@ -311,7 +315,7 @@ ReceiveRequest = CONTAINING_RECORD(CurrentEntry, DATAGRAM_RECEIVE_REQUEST, ListEntry); /* Abort the request and free its resources */ TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql); - (*ReceiveRequest->Complete)(ReceiveRequest->Context, STATUS_ADDRESS_CLOSED, 0); + (*ReceiveRequest->Complete)(ReceiveRequest->Context, STATUS_CANCELLED, 0); TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql); CurrentEntry = NextEntry; } @@ -326,42 +330,13 @@ DATAGRAM_SEND_REQUEST, ListEntry); /* Abort the request and free its resources */ TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql); - (*SendRequest->Complete)(SendRequest->Context, STATUS_ADDRESS_CLOSED, 0); + (*SendRequest->Complete)(SendRequest->Context, STATUS_CANCELLED, 0); exFreePool(SendRequest); TcpipAcquireSpinLock(&AddrFile->Lock, &OldIrql); CurrentEntry = NextEntry; }
TcpipReleaseSpinLock(&AddrFile->Lock, OldIrql); - - TI_DbgPrint(MAX_TRACE, ("Leaving.\n")); - - return Status; -} - - -/* - * FUNCTION: Closes an address file object - * ARGUMENTS: - * Request = Pointer to TDI request structure for this request - * RETURNS: - * Status of operation - */ -NTSTATUS FileFreeAddress( - PTDI_REQUEST Request) -{ - PADDRESS_FILE AddrFile; - NTSTATUS Status = STATUS_SUCCESS; - KIRQL OldIrql; - - AddrFile = Request->Handle.AddressHandle; - - TI_DbgPrint(MID_TRACE, ("Called.\n")); - - /* Remove address file from the global list */ - TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql); - RemoveEntryList(&AddrFile->ListEntry); - TcpipReleaseSpinLock(&AddressFileListLock, OldIrql);
/* Protocol specific handling */ switch (AddrFile->Protocol) { @@ -474,42 +449,20 @@ PTDI_REQUEST Request) { PCONNECTION_ENDPOINT Connection; + KIRQL OldIrql;
TI_DbgPrint(MID_TRACE, ("Called.\n"));
Connection = Request->Handle.ConnectionContext; + + TcpipAcquireSpinLock(&ConnectionEndpointListLock, &OldIrql); + RemoveEntryList(&Connection->ListEntry); + TcpipReleaseSpinLock(&ConnectionEndpointListLock, OldIrql);
TcpipRecursiveMutexEnter( &TCPLock, TRUE ); TCPClose( Connection ); TcpipRecursiveMutexLeave( &TCPLock );
- TI_DbgPrint(MAX_TRACE, ("Leaving.\n")); - - return STATUS_SUCCESS; -} - - -/* - * FUNCTION: Frees an connection file object - * ARGUMENTS: - * Request = Pointer to TDI request structure for this request - * RETURNS: - * Status of operation - */ -NTSTATUS FileFreeConnection( - PTDI_REQUEST Request) -{ - KIRQL OldIrql; - PCONNECTION_ENDPOINT Connection; - - TI_DbgPrint(MID_TRACE, ("Called.\n")); - - Connection = Request->Handle.ConnectionContext; - - TcpipAcquireSpinLock(&ConnectionEndpointListLock, &OldIrql); - RemoveEntryList(&Connection->ListEntry); - TcpipReleaseSpinLock(&ConnectionEndpointListLock, OldIrql); - TCPFreeConnectionEndpoint(Connection);
TI_DbgPrint(MAX_TRACE, ("Leaving.\n")); @@ -565,7 +518,7 @@ * RETURNS: * Status of operation */ -NTSTATUS FileFreeControlChannel( +NTSTATUS FileCloseControlChannel( PTDI_REQUEST Request) { PCONTROL_CHANNEL ControlChannel = Request->Handle.ControlChannel;
Modified: trunk/reactos/drivers/network/tcpip/tcpip/main.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip... ============================================================================== --- trunk/reactos/drivers/network/tcpip/tcpip/main.c [iso-8859-1] (original) +++ trunk/reactos/drivers/network/tcpip/tcpip/main.c [iso-8859-1] Tue Aug 4 20:56:56 2009 @@ -252,74 +252,6 @@ return Status; }
-/* - * FUNCTION: Prepares a file object for close - * ARGUMENTS: - * DeviceObject = Pointer to a device object for this driver - * Irp = Pointer to a I/O request packet - * RETURNS: - * Status of the operation - * NOTES: - * This function does not pend - */ -NTSTATUS TiCleanupFileObject( - PDEVICE_OBJECT DeviceObject, - PIRP Irp) -{ - PIO_STACK_LOCATION IrpSp; - PTRANSPORT_CONTEXT Context; - TDI_REQUEST Request; - NTSTATUS Status; - KIRQL OldIrql; - - IrpSp = IoGetCurrentIrpStackLocation(Irp); - Context = IrpSp->FileObject->FsContext; - if (!Context) { - TI_DbgPrint(MIN_TRACE, ("Parameters are invalid.\n")); - return STATUS_INVALID_PARAMETER; - } - - IoAcquireCancelSpinLock(&OldIrql); - - Context->CancelIrps = TRUE; - - IoReleaseCancelSpinLock(OldIrql); - - switch ((ULONG_PTR)IrpSp->FileObject->FsContext2) { - case TDI_TRANSPORT_ADDRESS_FILE: - Request.Handle.AddressHandle = Context->Handle.AddressHandle; - Status = FileCloseAddress(&Request); - break; - - case TDI_CONNECTION_FILE: - Request.Handle.ConnectionContext = Context->Handle.ConnectionContext; - Status = FileCloseConnection(&Request); - break; - - case TDI_CONTROL_CHANNEL_FILE: - Request.Handle.ControlChannel = Context->Handle.ControlChannel; - /* Nothing to do to close */ - Status = STATUS_SUCCESS; - break; - - default: - /* This should never happen */ - - TI_DbgPrint(MIN_TRACE, ("Unknown transport context.\n")); - - IoAcquireCancelSpinLock(&OldIrql); - Context->CancelIrps = FALSE; - IoReleaseCancelSpinLock(OldIrql); - - Status = STATUS_INVALID_PARAMETER; - - } - - Irp->IoStatus.Status = Status; - - return Irp->IoStatus.Status; -} -
/* * FUNCTION: Releases resources used by a file object @@ -339,7 +271,6 @@ PTRANSPORT_CONTEXT Context; TDI_REQUEST Request; NTSTATUS Status; - KIRQL OldIrql;
IrpSp = IoGetCurrentIrpStackLocation(Irp); Context = IrpSp->FileObject->FsContext; @@ -348,39 +279,26 @@ return STATUS_INVALID_PARAMETER; }
- IoAcquireCancelSpinLock(&OldIrql); - - Context->CancelIrps = TRUE; - - IoReleaseCancelSpinLock(OldIrql); - switch ((ULONG_PTR)IrpSp->FileObject->FsContext2) { case TDI_TRANSPORT_ADDRESS_FILE: Request.Handle.AddressHandle = Context->Handle.AddressHandle; - Status = FileFreeAddress(&Request); + Status = FileCloseAddress(&Request); break;
case TDI_CONNECTION_FILE: Request.Handle.ConnectionContext = Context->Handle.ConnectionContext; - Status = FileFreeConnection(&Request); + Status = FileCloseConnection(&Request); break;
case TDI_CONTROL_CHANNEL_FILE: Request.Handle.ControlChannel = Context->Handle.ControlChannel; - Status = FileFreeControlChannel(&Request); + Status = FileCloseControlChannel(&Request); break;
default: - /* This should never happen */ - - TI_DbgPrint(MIN_TRACE, ("Unknown transport context.\n")); - - IoAcquireCancelSpinLock(&OldIrql); - Context->CancelIrps = FALSE; - IoReleaseCancelSpinLock(OldIrql); - + DbgPrint("Unknown type %d\n", (ULONG_PTR)IrpSp->FileObject->FsContext2); Status = STATUS_INVALID_PARAMETER; - + break; }
Irp->IoStatus.Status = Status; @@ -422,12 +340,6 @@ case IRP_MJ_CLOSE: Context = (PTRANSPORT_CONTEXT)IrpSp->FileObject->FsContext; Status = TiCloseFileObject(DeviceObject, Irp); - break; - - /* Release resources bound to an address file, connection endpoint, - or control connection */ - case IRP_MJ_CLEANUP: - Status = TiCleanupFileObject(DeviceObject, Irp); break;
default: @@ -912,7 +824,6 @@ /* Initialize the driver object with this driver's entry points */ DriverObject->MajorFunction[IRP_MJ_CREATE] = TiDispatchOpenClose; DriverObject->MajorFunction[IRP_MJ_CLOSE] = TiDispatchOpenClose; - DriverObject->MajorFunction[IRP_MJ_CLEANUP] = TiDispatchOpenClose; DriverObject->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = TiDispatchInternal; DriverObject->MajorFunction[IRP_MJ_DEVICE_CONTROL] = TiDispatch;
-
cgutman＠svn.reactos.org