[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

svn path=/trunk/; revision=52086

reactos/drivers/network/tcpip/include/titypes.h

@@ -270,6 +270,8 @@ typedef struct _CONNECTION_ENDPOINT {
 
     /* Signals */
     UINT    SignalState;       /* Active signals from oskit */
+
+    struct _CONNECTION_ENDPOINT *Next; /* Next connection in address file list */
 } CONNECTION_ENDPOINT, *PCONNECTION_ENDPOINT;
 
 

reactos/drivers/network/tcpip/tcpip/dispatch.c

@@ -259,7 +259,7 @@ NTSTATUS DispTdiAssociateAddress(
   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 @@ NTSTATUS DispTdiAssociateAddress(
 
   /* Add connection endpoint to the address file */
   ReferenceObject(Connection);
+  if (AddrFile->Connection == NULL)
       AddrFile->Connection = Connection;
+  else
+  {
+      LastConnection = AddrFile->Connection;
+      while (LastConnection->Next != NULL)
+         LastConnection = LastConnection->Next;
+      LastConnection->Next = Connection;
+  }
 
-  /* FIXME: Maybe do this in DispTdiDisassociateAddress() instead? */
   ObDereferenceObject(FileObject);
 
   UnlockObjectFromDpcLevel(AddrFile);
   UnlockObject(Connection, OldIrql);
 
-  return Status;
+  return STATUS_SUCCESS;
 }
 
 
@@ -425,10 +432,11 @@ NTSTATUS DispTdiDisassociateAddress(
  *     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 @@ NTSTATUS DispTdiDisassociateAddress(
 
   LockObjectAtDpcLevel(Connection->AddressFile);
 
-  /* Remove this connection from the address file */
+  /* Unlink this connection from the address file */
-  DereferenceObject(Connection->AddressFile->Connection);
+  if (Connection->AddressFile->Connection == Connection)
-  Connection->AddressFile->Connection = NULL;
+  {
+      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);
 
+  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 @@ NTSTATUS DispTdiListen(
 	  Status = STATUS_NO_MEMORY;
 
       if( NT_SUCCESS(Status) ) {
-          ReferenceObject(Connection->AddressFile);
 	  Connection->AddressFile->Listener->AddressFile =
 	      Connection->AddressFile;
 

reactos/drivers/network/tcpip/tcpip/fileobjs.c

@@ -161,6 +161,9 @@ VOID AddrFileFree(
 
   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 @@ NTSTATUS FileCloseAddress(
   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);

reactos/lib/drivers/ip/transport/tcp/tcp.c

@@ -666,13 +666,14 @@ NTSTATUS TCPClose
     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 @@ NTSTATUS TCPClose
        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;
 }