[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
4 Aug
2009
4 Aug
'09
6:56 p.m.
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/bi…
==============================================================================
--- 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/co…
==============================================================================
--- 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/ma…
==============================================================================
--- 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/td…
==============================================================================
--- 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/incl…
==============================================================================
--- 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/tcpi…
==============================================================================
--- 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/tcpi…
==============================================================================
--- 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;
5623
days inactive
5623
days old
0 comments
1 participants
participants (1)
-
cgutman＠svn.reactos.org