From 0553d5160cf9d0731e6ae59f5b813b81b3795f58 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Thu, 26 Aug 2010 02:29:38 +0000 Subject: [PATCH] [OSKITTCP] - Prevent multiple wakeups for the same event which caused nasty problems for the SEL_FIN event because we dereferenced our connection context 3 times which not only caused the connection endpoint to be freed while holding its spin lock but made the reference count negative [TCPIP] - Disassociate the address file from the connection endpoint before dereferencing/closing it to avoid a double dereference of the address file (not as harmful in this case as in the connection endpoint case) [IP] - Dereference the connection endpoint again if it was associated with an address file as the connection endpoint to fix a reference leak svn path=/trunk/; revision=48624 --- .../drivers/network/tcpip/tcpip/fileobjs.c | 6 ++++ reactos/lib/drivers/ip/transport/tcp/tcp.c | 28 +++++++++++++++---- .../drivers/oskittcp/oskittcp/uipc_socket2.c | 8 ++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/reactos/drivers/network/tcpip/tcpip/fileobjs.c b/reactos/drivers/network/tcpip/tcpip/fileobjs.c index 70357fced9a..98cc4062d0d 100644 --- a/reactos/drivers/network/tcpip/tcpip/fileobjs.c +++ b/reactos/drivers/network/tcpip/tcpip/fileobjs.c @@ -379,9 +379,15 @@ NTSTATUS FileCloseAddress( LockObject(AddrFile, &OldIrql); /* We have to close this connection 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); diff --git a/reactos/lib/drivers/ip/transport/tcp/tcp.c b/reactos/lib/drivers/ip/transport/tcp/tcp.c index b432ab83f79..e980f6b1e15 100644 --- a/reactos/lib/drivers/ip/transport/tcp/tcp.c +++ b/reactos/lib/drivers/ip/transport/tcp/tcp.c @@ -234,7 +234,10 @@ VOID HandleSignalledConnection(PCONNECTION_ENDPOINT Connection) /* If the socket is dead, remove the reference we added for oskit */ if (Connection->SignalState & SEL_FIN) + { + Connection->SocketContext = NULL; DereferenceObject(Connection); + } } VOID ConnectionFree(PVOID Object) { @@ -663,17 +666,15 @@ NTSTATUS TCPClose KIRQL OldIrql; NTSTATUS Status; PVOID Socket; + PADDRESS_FILE AddressFile = NULL; + PCONNECTION_ENDPOINT AddressConnection = NULL; - /* We don't rely on SocketContext == NULL for socket - * closure anymore but we still need it to determine - * if we caused the closure - */ LockObject(Connection, &OldIrql); Socket = Connection->SocketContext; Connection->SocketContext = NULL; /* Don't try to close again if the other side closed us already */ - if (!(Connection->SignalState & SEL_FIN)) + if (Socket) { /* We need to close here otherwise oskit will never indicate * SEL_FIN and we will never fully close the connection */ @@ -693,11 +694,26 @@ NTSTATUS TCPClose } if (Connection->AddressFile) - DereferenceObject(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; } diff --git a/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c b/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c index d565910f289..4155922d804 100644 --- a/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c +++ b/reactos/lib/drivers/oskittcp/oskittcp/uipc_socket2.c @@ -118,8 +118,10 @@ soisconnected(so) wakeup(so, (caddr_t)&head->so_timeo); } else { wakeup(so, (caddr_t)&so->so_timeo); +#ifndef __REACTOS__ sorwakeup(so); sowwakeup(so); +#endif } } @@ -131,8 +133,10 @@ soisdisconnecting(so) so->so_state &= ~SS_ISCONNECTING; so->so_state |= (SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE); wakeup(so, (caddr_t)&so->so_timeo); +#ifndef __REACTOS__ sowwakeup(so); sorwakeup(so); +#endif } void @@ -144,8 +148,10 @@ soisdisconnected(so) so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING); so->so_state |= (SS_CANTRCVMORE|SS_CANTSENDMORE); wakeup(so, (caddr_t)&so->so_timeo); +#ifndef __REACTOS__ sowwakeup(so); sorwakeup(so); +#endif } /* @@ -192,7 +198,9 @@ sonewconn1(head, connstatus) return ((struct socket *)0); } if (connstatus) { +#ifndef __REACTOS__ sorwakeup(head); +#endif wakeup(head, (caddr_t)&head->so_timeo); so->so_state |= connstatus; }