[ros-diffs] [cgutman] 52086: [IP/TCPIP] - Add a Next member to CONNECTION_ENDPOINT to allow multiple connections to be associated with a single address file while not overwriting pointers, dereferencing other ...
cgutman at svn.reactos.org
cgutman at svn.reactos.org
Sun Jun 5 02:16:47 UTC 2011
Author: cgutman
Date: Sun Jun 5 02:16:45 2011
New Revision: 52086
URL: http://svn.reactos.org/svn/reactos?rev=52086&view=rev
Log:
[IP/TCPIP]
- Add a Next member to CONNECTION_ENDPOINT to allow multiple connections to be associated with a single address file while not overwriting pointers, dereferencing other people's connection objects, and causing horrific amounts of memory corruption
- Add several sanity checks to prevent this from happening again
- Don't try dereference address files and connection endpoints in the free functions; there should be none associated since r52083 (sanity checks ensure this)
- Don't hold an extra reference to the address file when creating a listener; this reference is implicit
- This should greatly increase reliability of activities that open lots of sockets such as web browsing and running servers on ROS
- This also fixes most issues of not releasing a server port when the listener is closed
Modified:
trunk/reactos/drivers/network/tcpip/include/titypes.h
trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c
trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c
trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c
Modified: trunk/reactos/drivers/network/tcpip/include/titypes.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/include/titypes.h?rev=52086&r1=52085&r2=52086&view=diff
==============================================================================
--- trunk/reactos/drivers/network/tcpip/include/titypes.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/include/titypes.h [iso-8859-1] Sun Jun 5 02:16:45 2011
@@ -270,6 +270,8 @@
/* Signals */
UINT SignalState; /* Active signals from oskit */
+
+ struct _CONNECTION_ENDPOINT *Next; /* Next connection in address file list */
} CONNECTION_ENDPOINT, *PCONNECTION_ENDPOINT;
Modified: trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c?rev=52086&r1=52085&r2=52086&view=diff
==============================================================================
--- trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/tcpip/dispatch.c [iso-8859-1] Sun Jun 5 02:16:45 2011
@@ -259,7 +259,7 @@
PTDI_REQUEST_KERNEL_ASSOCIATE Parameters;
PTRANSPORT_CONTEXT TranContext;
PIO_STACK_LOCATION IrpSp;
- PCONNECTION_ENDPOINT Connection;
+ PCONNECTION_ENDPOINT Connection, LastConnection;
PFILE_OBJECT FileObject;
PADDRESS_FILE AddrFile = NULL;
NTSTATUS Status;
@@ -340,15 +340,22 @@
/* Add connection endpoint to the address file */
ReferenceObject(Connection);
- AddrFile->Connection = Connection;
-
- /* FIXME: Maybe do this in DispTdiDisassociateAddress() instead? */
+ if (AddrFile->Connection == NULL)
+ AddrFile->Connection = Connection;
+ else
+ {
+ LastConnection = AddrFile->Connection;
+ while (LastConnection->Next != NULL)
+ LastConnection = LastConnection->Next;
+ LastConnection->Next = Connection;
+ }
+
ObDereferenceObject(FileObject);
UnlockObjectFromDpcLevel(AddrFile);
UnlockObject(Connection, OldIrql);
- return Status;
+ return STATUS_SUCCESS;
}
@@ -425,10 +432,11 @@
* Status of operation
*/
{
- PCONNECTION_ENDPOINT Connection;
+ PCONNECTION_ENDPOINT Connection, LastConnection;
PTRANSPORT_CONTEXT TranContext;
PIO_STACK_LOCATION IrpSp;
KIRQL OldIrql;
+ NTSTATUS Status;
TI_DbgPrint(DEBUG_IRP, ("Called.\n"));
@@ -458,19 +466,42 @@
LockObjectAtDpcLevel(Connection->AddressFile);
- /* Remove this connection from the address file */
- DereferenceObject(Connection->AddressFile->Connection);
- Connection->AddressFile->Connection = NULL;
+ /* Unlink this connection from the address file */
+ if (Connection->AddressFile->Connection == Connection)
+ {
+ Connection->AddressFile->Connection = Connection->Next;
+ DereferenceObject(Connection);
+ Status = STATUS_SUCCESS;
+ }
+ else
+ {
+ LastConnection = Connection->AddressFile->Connection;
+ while (LastConnection->Next != Connection && LastConnection->Next != NULL)
+ LastConnection = LastConnection->Next;
+ if (LastConnection->Next == Connection)
+ {
+ LastConnection->Next = Connection->Next;
+ DereferenceObject(Connection);
+ Status = STATUS_SUCCESS;
+ }
+ else
+ {
+ Status = STATUS_INVALID_PARAMETER;
+ }
+ }
UnlockObjectFromDpcLevel(Connection->AddressFile);
- /* Remove the address file from this connection */
- DereferenceObject(Connection->AddressFile);
- Connection->AddressFile = NULL;
+ if (Status == STATUS_SUCCESS)
+ {
+ /* Remove the address file from this connection */
+ DereferenceObject(Connection->AddressFile);
+ Connection->AddressFile = NULL;
+ }
UnlockObject(Connection, OldIrql);
- return STATUS_SUCCESS;
+ return Status;
}
@@ -602,7 +633,6 @@
Status = STATUS_NO_MEMORY;
if( NT_SUCCESS(Status) ) {
- ReferenceObject(Connection->AddressFile);
Connection->AddressFile->Listener->AddressFile =
Connection->AddressFile;
Modified: trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c?rev=52086&r1=52085&r2=52086&view=diff
==============================================================================
--- trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c [iso-8859-1] Sun Jun 5 02:16:45 2011
@@ -161,6 +161,9 @@
TI_DbgPrint(MID_TRACE, ("Called.\n"));
+ /* We should not be associated with a connection here */
+ ASSERT(!AddrFile->Connection);
+
/* Remove address file from the global list */
TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql);
RemoveEntryList(&AddrFile->ListEntry);
@@ -377,17 +380,14 @@
if (!Request->Handle.AddressHandle) return STATUS_INVALID_PARAMETER;
LockObject(AddrFile, &OldIrql);
- /* We have to close this connection because we started it */
+
+ /* We have to close this listener because we started it */
if( AddrFile->Listener )
{
AddrFile->Listener->AddressFile = NULL;
TCPClose( AddrFile->Listener );
}
- if( AddrFile->Connection )
- {
- AddrFile->Connection->AddressFile = NULL;
- DereferenceObject( AddrFile->Connection );
- }
+
UnlockObject(AddrFile, OldIrql);
DereferenceObject(AddrFile);
Modified: trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c?rev=52086&r1=52085&r2=52086&view=diff
==============================================================================
--- trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/transport/tcp/tcp.c [iso-8859-1] Sun Jun 5 02:16:45 2011
@@ -666,12 +666,13 @@
KIRQL OldIrql;
NTSTATUS Status;
PVOID Socket;
- PADDRESS_FILE AddressFile = NULL;
- PCONNECTION_ENDPOINT AddressConnection = NULL;
LockObject(Connection, &OldIrql);
Socket = Connection->SocketContext;
Connection->SocketContext = NULL;
+
+ /* We should not be associated to an address file at this point */
+ ASSERT(!Connection->AddressFile);
/* Don't try to close again if the other side closed us already */
if (Socket)
@@ -693,27 +694,9 @@
Status = STATUS_SUCCESS;
}
- if (Connection->AddressFile)
- {
- LockObjectAtDpcLevel(Connection->AddressFile);
- if (Connection->AddressFile->Connection == Connection)
- {
- AddressConnection = Connection->AddressFile->Connection;
- Connection->AddressFile->Connection = NULL;
- }
- UnlockObjectFromDpcLevel(Connection->AddressFile);
-
- AddressFile = Connection->AddressFile;
- Connection->AddressFile = NULL;
- }
-
UnlockObject(Connection, OldIrql);
DereferenceObject(Connection);
- if (AddressConnection)
- DereferenceObject(AddressConnection);
- if (AddressFile)
- DereferenceObject(AddressFile);
return Status;
}
More information about the Ros-diffs
mailing list