Fix refcounting/tracking of NBCopies. Check comments for explanations

Freepuxian · Jul 30, 2020 · af9940a · af9940a
1 parent b039732
commit af9940a
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 18 deletions.
diff --git a/packetWin7/npf/npf/ObjPool.c b/packetWin7/npf/npf/ObjPool.c
@@ -48,6 +48,7 @@
  ***************************************************************************/
 #include "Packet.h"
 #include "ObjPool.h"
+#include <limits.h>
 
 typedef struct _NPF_OBJ_SHELF
 {
@@ -282,6 +283,7 @@ VOID NPF_ObjectPoolReturn(PVOID pObject, PNPF_OBJ_CLEANUP CleanupFunc, BOOLEAN b
 	PNPF_OBJ_POOL pPool = NULL;
 	PNPF_OBJ_POOL_ELEM pElem = CONTAINING_RECORD(pObject, NPF_OBJ_POOL_ELEM, pObject);
 	ULONG refcount = InterlockedDecrement(&pElem->Refcount);
+	ASSERT(refcount < ULONG_MAX);
 
 	if (refcount == 0)
 	{

diff --git a/packetWin7/npf/npf/Openclos.c b/packetWin7/npf/npf/Openclos.c
@@ -464,18 +464,6 @@ VOID NPF_FreeNBCopies(PNPF_NB_COPIES pNBCopy, BOOLEAN bAtDispatchLevel)
 _Use_decl_annotations_
 VOID NPF_FreeNBLCopy(PNPF_NBL_COPY pNBLCopy, BOOLEAN bAtDispatchLevel)
 {
-	PNPF_NB_COPIES pNBCopies = NULL;
-	PSINGLE_LIST_ENTRY pNBCopiesEntry = NULL;
-
-	pNBCopiesEntry = pNBLCopy->NBCopiesHead.Next;
-	while (pNBCopiesEntry != NULL)
-	{
-		pNBCopies = CONTAINING_RECORD(pNBCopiesEntry, NPF_NB_COPIES, CopiesEntry);
-		pNBCopiesEntry = pNBCopiesEntry->Next;
-
-		NPF_ObjectPoolReturn(pNBCopies, NPF_FreeNBCopies, bAtDispatchLevel);
-	}
-
 	if (pNBLCopy->Dot11RadiotapHeader != NULL)
 	{
 		NPF_ObjectPoolReturn(pNBLCopy->Dot11RadiotapHeader, NULL, bAtDispatchLevel);
@@ -2633,7 +2621,7 @@ NOTE: Called at PASSIVE_LEVEL and the filter is in paused state
 
 	TRACE_ENTER();
 
-	ASSERT(pFiltMod->AdapterBindingStatus == FilterPaused);
+	ASSERT(pFiltMod->AdapterBindingStatus == FilterPaused || pFiltMod->Loopback);
 	/* No need to lock the group since we are paused. */
 	for (Curr = pFiltMod->OpenInstances.Next; Curr != NULL; Curr = Curr->Next)
 	{

diff --git a/packetWin7/npf/npf/Packet.h b/packetWin7/npf/npf/Packet.h
@@ -469,6 +469,7 @@ typedef struct _NPF_NB_COPIES
 	PNET_BUFFER pNetBuffer; // May be NULL, hence why we can't just use NET_BUFFER.Next
 	ULONG ulSize; //Size of all allocated space in the netbuffer.
 	ULONG ulPacketSize; // Size of the original packet
+	ULONG ulRefcount; // How many NPF_CAP_DATA are using this copy
 } NPF_NB_COPIES, *PNPF_NB_COPIES;
 
 VOID NPF_FreeNBCopies(_In_ PNPF_NB_COPIES pNBCopy, _In_ BOOLEAN bAtDispatchLevel);

diff --git a/packetWin7/npf/npf/Read.c b/packetWin7/npf/npf/Read.c
@@ -385,9 +385,11 @@ NPF_Read(
 		// Increase free space by the amount that it was reduced before
 		InterlockedExchangeAdd(&Open->Free, NPF_CAP_SIZE(pCapData, pRadiotapHeader));
 
-		// If the NBCopy has data buffers, refcount it and push it onto the cache stack
-		if (pCapData->pNBCopy->ulSize > NPF_NBCOPY_INITIAL_DATA_SIZE)
+		// If we are the last one using this NBCopy and it has data buffers,
+		if (0 == InterlockedDecrement(&pCapData->pNBCopy->ulRefcount)
+				&& pCapData->pNBCopy->ulSize > NPF_NBCOPY_INITIAL_DATA_SIZE)
 		{
+			// refcount it and push it onto the cache stack
 			NPF_ReferenceObject(pCapData->pNBCopy);
 			ExInterlockedPushEntryList(&Open->DeviceExtension->NBCopiesCache,
 					&pCapData->pNBCopy->CopiesEntry,
@@ -442,6 +444,8 @@ NPF_DoTap(
 	SINGLE_LIST_ENTRY NBLCopiesHead;
 	struct timeval tstamp = {0, 0};
 	NBLCopiesHead.Next = NULL;
+	PNPF_NB_COPIES pNBCopies = NULL;
+	PSINGLE_LIST_ENTRY pNBCopiesEntry = NULL;
 
 	/* Lock the group */
 	// Read-only lock since list is not being modified.
@@ -467,6 +471,14 @@ NPF_DoTap(
 	for (Curr = NBLCopiesHead.Next; Curr != NULL; Curr = Curr->Next)
 	{
 		pNBLCopy = CONTAINING_RECORD(Curr, NPF_NBL_COPY, NBLCopyEntry); 
+		pNBCopiesEntry = pNBLCopy->NBCopiesHead.Next;
+		while (pNBCopiesEntry != NULL)
+		{
+			pNBCopies = CONTAINING_RECORD(pNBCopiesEntry, NPF_NB_COPIES, CopiesEntry);
+			pNBCopiesEntry = pNBCopiesEntry->Next;
+
+			NPF_ObjectPoolReturn(pNBCopies, NPF_FreeNBCopies, AtDispatchLevel);
+		}
 		NPF_ObjectPoolReturn(pNBLCopy, NPF_FreeNBLCopy, AtDispatchLevel);
 	}
 
@@ -608,6 +620,9 @@ PNPF_NB_COPIES NPF_GetNBCopy(
 	else
 	{
 		pNBCopy = CONTAINING_RECORD(pCopiesEntry, NPF_NB_COPIES, CopiesEntry);
+		// We use the same list entry object for chaining copies in an NBLCopy,
+		// so we have to ensure Next is NULL for "new" copies.
+		pCopiesEntry->Next = NULL;
 	}
 
 	pNetBuffer = pNBCopy->pNetBuffer;
@@ -1103,6 +1118,7 @@ NPF_TapExForEachOpen(
 			}
 			// Increment refcounts on relevant structures
 			pCapData->pNBCopy = pNBCopy;
+			InterlockedIncrement(&pNBCopy->ulRefcount);
 			NPF_ReferenceObject(pNBCopy);
 			NPF_ReferenceObject(pNBLCopy);
 

diff --git a/version.h b/version.h
@@ -87,11 +87,11 @@
 #define /*
  !define /**/ WINPCAP_MINOR				0
 #define /*
- !define /**/ WINPCAP_REV				9995
+ !define /**/ WINPCAP_REV				9996
 #define /*
- !define /**/ WINPCAP_BUILD				709
+ !define /**/ WINPCAP_BUILD				729
 #define /*
- !define /**/ WINPCAP_VER_STRING		"0.9995"
+ !define /**/ WINPCAP_VER_STRING		"0.9996"
 
 #define WINPCAP_WPCAP_STRING_VERSION WINPCAP_VER_STRING