Author: cgutman
Date: Sun Aug 17 04:03:29 2014
New Revision: 63899
URL: http://svn.reactos.org/svn/reactos?rev=63899&view=rev
Log:
[TCPIP]
- Reference the address file while delivering data to avoid a use after free when an address file is closed during datagram delivery
Modified:
trunk/reactos/drivers/network/tcpip/tcpip/fileobjs.c
trunk/reactos/lib/drivers/ip/transport/rawip/rawip.c
trunk/reactos/lib/drivers/ip/transport/udp/udp.c
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] Sun Aug 17 04:03:29 2014
@@ -222,7 +222,7 @@
* ARGUMENTS:
* SearchContext = Pointer to search context
* RETURNS:
- * Pointer to address file, NULL if none was found
+ * Pointer to referenced address file, NULL if none was found
*/
PADDRESS_FILE AddrSearchNext(
PAF_SEARCH SearchContext)
@@ -232,6 +232,7 @@
KIRQL OldIrql;
PADDRESS_FILE Current = NULL;
BOOLEAN Found = FALSE;
+ PADDRESS_FILE StartingAddrFile;
TcpipAcquireSpinLock(&AddressFileListLock, &OldIrql);
@@ -241,8 +242,8 @@
return NULL;
}
- /* Remove the extra reference we added to keep this address file in memory */
- DereferenceObject(CONTAINING_RECORD(SearchContext->Next, ADDRESS_FILE, ListEntry));
+ /* Save this pointer so we can dereference it later */
+ StartingAddrFile = CONTAINING_RECORD(SearchContext->Next, ADDRESS_FILE, ListEntry);
CurrentEntry = SearchContext->Next;
@@ -279,9 +280,15 @@
/* Reference the next address file to prevent the link from disappearing behind our back */
ReferenceObject(CONTAINING_RECORD(SearchContext->Next, ADDRESS_FILE, ListEntry));
}
+
+ /* Reference the returned address file before dereferencing the starting
+ * address file because it may be that Current == StartingAddrFile */
+ ReferenceObject(Current);
}
else
Current = NULL;
+
+ DereferenceObject(StartingAddrFile);
TcpipReleaseSpinLock(&AddressFileListLock, OldIrql);
Modified: trunk/reactos/lib/drivers/ip/transport/rawip/rawip.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/transport/r…
==============================================================================
--- trunk/reactos/lib/drivers/ip/transport/rawip/rawip.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/transport/rawip/rawip.c [iso-8859-1] Sun Aug 17 04:03:29 2014
@@ -321,6 +321,7 @@
0,
IPPacket,
DataSize);
+ DereferenceObject(AddrFile);
} while ((AddrFile = AddrSearchNext(&SearchContext)) != NULL);
} else {
/* There are no open address files that will take this datagram */
Modified: trunk/reactos/lib/drivers/ip/transport/udp/udp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/drivers/ip/transport/u…
==============================================================================
--- trunk/reactos/lib/drivers/ip/transport/udp/udp.c [iso-8859-1] (original)
+++ trunk/reactos/lib/drivers/ip/transport/udp/udp.c [iso-8859-1] Sun Aug 17 04:03:29 2014
@@ -320,6 +320,7 @@
UDPHeader->DestPort,
IPPacket,
DataSize);
+ DereferenceObject(AddrFile);
} while ((AddrFile = AddrSearchNext(&SearchContext)) != NULL);
} else {
/* There are no open address files that will take this datagram */
Author: cgutman
Date: Sun Aug 17 01:42:02 2014
New Revision: 63898
URL: http://svn.reactos.org/svn/reactos?rev=63898&view=rev
Log:
[HAL]
Fix a catastrophic bug in S/G DMA. There is a subtle difference between the S/G DMA APIs and the old AllocateAdapterChannel API when it comes to having multiple requests in flight. Callers of (Io)AllocateAdapterChannel CANNOT queue another request until the AdapterControlRoutine is called. S/G DMA allows multiple concurrent DMA requests, but ROS was using IoAllocateAdapterChannel in the S/G API. As a result, the wait block stored in the device object was unexpectedly reinitalized and queued again. This results in a leak of the originally queued request context, potentially performing the new DMA operation twice while dropping the old request, and use after free of the context passed to HalpScatterGatherAdapterControl.
Modified:
trunk/reactos/hal/halx86/generic/dma.c
Modified: trunk/reactos/hal/halx86/generic/dma.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/generic/dma.c?r…
==============================================================================
--- trunk/reactos/hal/halx86/generic/dma.c [iso-8859-1] (original)
+++ trunk/reactos/hal/halx86/generic/dma.c [iso-8859-1] Sun Aug 17 01:42:02 2014
@@ -919,6 +919,7 @@
PVOID AdapterListControlContext, MapRegisterBase;
ULONG MapRegisterCount;
BOOLEAN WriteToDevice;
+ WAIT_CONTEXT_BLOCK Wcb;
} SCATTER_GATHER_CONTEXT, *PSCATTER_GATHER_CONTEXT;
@@ -1041,11 +1042,14 @@
AdapterControlContext->AdapterListControlContext = Context;
AdapterControlContext->WriteToDevice = WriteToDevice;
- return IoAllocateAdapterChannel(AdapterObject,
- DeviceObject,
- AdapterControlContext->MapRegisterCount,
- HalpScatterGatherAdapterControl,
- AdapterControlContext);
+ AdapterControlContext->Wcb.DeviceObject = DeviceObject;
+ AdapterControlContext->Wcb.DeviceContext = AdapterControlContext;
+ AdapterControlContext->Wcb.CurrentIrp = DeviceObject->CurrentIrp;
+
+ return HalAllocateAdapterChannel(AdapterObject,
+ &AdapterControlContext->Wcb,
+ AdapterControlContext->MapRegisterCount,
+ HalpScatterGatherAdapterControl);
}
/**