From 99913119c19b2742242ecd6d5c4e69b7753b1126 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Fri, 29 Nov 2024 13:32:27 +0200 Subject: [PATCH 1/9] netkvm: deregister miniport in erroneous path https://issues.redhat.com/browse/RHEL-68725 The problem can be reproduced under driver verifier with low resource simulation. If the miniport is registered but the protocol fail to register we need to deregister the miniport. Otherwise in future attempts to load the driver the NDIS enters erroneous state and may issue BSOD. Example of BSOD: BUGCODE_NDIS_DRIVER/NDIS_BUGCHECK_REFCOUNT_IMBALANCE. Signed-off-by: Yuri Benditovich --- NetKVM/wlh/ParaNdis6_Driver.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 23533d4bd..849eb661b 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -1199,6 +1199,10 @@ NTSTATUS DriverEntry(PDRIVER_OBJECT pDriverObject, PUNICODE_STRING pRegistryPath else { DEBUG_EXIT_STATUS(0, status); + if (DriverHandle) + { + NdisMDeregisterMiniportDriver(DriverHandle); + } ParaNdis_DebugCleanup(pDriverObject); #ifdef NETKVM_WPP_ENABLED WPP_CLEANUP(pDriverObject); From 359de336ceb93eab353023a64eee7293f08d37a2 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Fri, 29 Nov 2024 16:44:09 +0200 Subject: [PATCH 2/9] netkvm: use per-queue max RX buffers instead of global Global value of max RX buffers, shared among queues, is bad idea. It is especially bad when memory allocation during queue initialization may fail, for example in case of driver running under driver verifier. Changing the global field to per-queue field. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-RX.h | 8 +++++--- NetKVM/Common/ParaNdis_Common.cpp | 2 +- NetKVM/Common/ParaNdis_Oid.cpp | 10 +++++++++- NetKVM/Common/ParaNdis_RX.cpp | 22 +++++++++++----------- NetKVM/Common/ndis56common.h | 2 +- NetKVM/wlh/ParaNdis6_Oid.cpp | 2 +- 6 files changed, 28 insertions(+), 18 deletions(-) diff --git a/NetKVM/Common/ParaNdis-RX.h b/NetKVM/Common/ParaNdis-RX.h index 8cba5125a..07796c339 100644 --- a/NetKVM/Common/ParaNdis-RX.h +++ b/NetKVM/Common/ParaNdis-RX.h @@ -39,14 +39,16 @@ class CParaNdisRX : public CParaNdisTemplatePath, public CNdisAlloca void KickRXRing(); PARANDIS_RECEIVE_QUEUE &UnclassifiedPacketsQueue() { return m_UnclassifiedPacketsQueue; } - + UINT GetFreeRxBuffers() const { return m_NetNofReceiveBuffers; } private: /* list of Rx buffers available for data (under VIRTIO management) */ LIST_ENTRY m_NetReceiveBuffers; - UINT m_NetNofReceiveBuffers; + UINT m_NetNofReceiveBuffers = 0; + UINT m_NetMaxReceiveBuffers = 0; UINT m_MinRxBufferLimit; - UINT m_nReusedRxBuffersCounter, m_nReusedRxBuffersLimit = 0; + UINT m_nReusedRxBuffersCounter = 0; + UINT m_nReusedRxBuffersLimit = 0; bool m_Reinsert = true; diff --git a/NetKVM/Common/ParaNdis_Common.cpp b/NetKVM/Common/ParaNdis_Common.cpp index df60cb62c..40359acd7 100755 --- a/NetKVM/Common/ParaNdis_Common.cpp +++ b/NetKVM/Common/ParaNdis_Common.cpp @@ -287,7 +287,7 @@ static void ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddre virtioDebugLevel = pConfiguration->debugLevel.ulValue; pContext->physicalMediaType = (NDIS_PHYSICAL_MEDIUM)pConfiguration->PhysicalMediaType.ulValue; pContext->maxFreeTxDescriptors = pConfiguration->TxCapacity.ulValue; - pContext->NetMaxReceiveBuffers = pConfiguration->RxCapacity.ulValue; + pContext->maxRxBufferPerQueue = pConfiguration->RxCapacity.ulValue; pContext->uNumberOfHandledRXPacketsInDPC = pConfiguration->NumberOfHandledRXPacketsInDPC.ulValue; pContext->bDoSupportPriority = pConfiguration->PrioritySupport.ulValue != 0; pContext->Offload.flagsValue = 0; diff --git a/NetKVM/Common/ParaNdis_Oid.cpp b/NetKVM/Common/ParaNdis_Oid.cpp index d311f825c..2fdca7fe1 100755 --- a/NetKVM/Common/ParaNdis_Oid.cpp +++ b/NetKVM/Common/ParaNdis_Oid.cpp @@ -273,8 +273,16 @@ NDIS_STATUS ParaNdis_OidQueryCommon(PARANDIS_ADAPTER *pContext, tOidDesc *pOid) break; } case OID_GEN_RECEIVE_BUFFER_SPACE: - SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx * pContext->NetMaxReceiveBuffers); + { + ULONG totalRxBuffer = 0; + for (UINT i = 0; i < pContext->nPathBundles; i++) + { + totalRxBuffer += pContext->pPathBundles[i].rxPath.GetFreeRxBuffers(); + } + + SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx * totalRxBuffer); break; + } case OID_GEN_RECEIVE_BLOCK_SIZE: SETINFO(ul, pContext->MaxPacketSize.nMaxFullSizeOsRx); break; diff --git a/NetKVM/Common/ParaNdis_RX.cpp b/NetKVM/Common/ParaNdis_RX.cpp index 088b38ab6..17863e0a7 100644 --- a/NetKVM/Common/ParaNdis_RX.cpp +++ b/NetKVM/Common/ParaNdis_RX.cpp @@ -72,7 +72,7 @@ static void ParaNdis_FreeRxBufferDescriptor(PARANDIS_ADAPTER *pContext, pRxNetDe NdisFreeMemory(p, 0, 0); } -CParaNdisRX::CParaNdisRX() : m_nReusedRxBuffersCounter(0), m_NetNofReceiveBuffers(0) +CParaNdisRX::CParaNdisRX() { InitializeListHead(&m_NetReceiveBuffers); } @@ -85,6 +85,7 @@ bool CParaNdisRX::Create(PPARANDIS_ADAPTER Context, UINT DeviceQueueIndex) { m_Context = Context; m_queueIndex = (u16)DeviceQueueIndex; + m_NetMaxReceiveBuffers = Context->maxRxBufferPerQueue; if (!m_VirtQueue.Create(DeviceQueueIndex, &m_Context->IODevice, @@ -96,7 +97,7 @@ bool CParaNdisRX::Create(PPARANDIS_ADAPTER Context, UINT DeviceQueueIndex) PrepareReceiveBuffers(); - m_nReusedRxBuffersLimit = m_Context->NetMaxReceiveBuffers / 4 + 1; + m_nReusedRxBuffersLimit = m_NetMaxReceiveBuffers / 4 + 1; CreatePath(); @@ -109,7 +110,7 @@ int CParaNdisRX::PrepareReceiveBuffers() UINT i; DEBUG_ENTRY(4); - for (i = 0; i < m_Context->NetMaxReceiveBuffers; ++i) + for (i = 0; i < m_NetMaxReceiveBuffers; ++i) { pRxNetDescriptor pBuffersDescriptor = CreateRxDescriptorOnInit(); if (!pBuffersDescriptor) break; @@ -126,10 +127,9 @@ int CParaNdisRX::PrepareReceiveBuffers() m_NetNofReceiveBuffers++; } - /* TODO - NetMaxReceiveBuffers should take into account all queues */ - m_Context->NetMaxReceiveBuffers = m_NetNofReceiveBuffers; + m_NetMaxReceiveBuffers = m_NetNofReceiveBuffers; m_MinRxBufferLimit = m_NetNofReceiveBuffers * m_Context->MinRxBufferPercent / 100; - DPrintf(0, "[%s] MaxReceiveBuffers %d, m_MinRxBufferLimit %u\n", __FUNCTION__, m_Context->NetMaxReceiveBuffers, m_MinRxBufferLimit); + DPrintf(0, "[%s] m_NetMaxReceiveBuffers %d, m_MinRxBufferLimit %u\n", __FUNCTION__, m_NetMaxReceiveBuffers, m_MinRxBufferLimit); if (m_Context->extraStatistics.minFreeRxBuffers == 0 || m_Context->extraStatistics.minFreeRxBuffers > m_NetNofReceiveBuffers) { m_Context->extraStatistics.minFreeRxBuffers = m_NetNofReceiveBuffers; @@ -240,10 +240,10 @@ void CParaNdisRX::ReuseReceiveBufferNoLock(pRxNetDescriptor pBuffersDescriptor) InsertTailList(&m_NetReceiveBuffers, &pBuffersDescriptor->listEntry); m_NetNofReceiveBuffers++; - if (m_NetNofReceiveBuffers > m_Context->NetMaxReceiveBuffers) + if (m_NetNofReceiveBuffers > m_NetMaxReceiveBuffers) { - DPrintf(0, " Error: NetNofReceiveBuffers > NetMaxReceiveBuffers(%d>%d)\n", - m_NetNofReceiveBuffers, m_Context->NetMaxReceiveBuffers); + DPrintf(0, " Error: m_NetNofReceiveBuffers > m_NetMaxReceiveBuffers (%d>%d)\n", + m_NetNofReceiveBuffers, m_NetMaxReceiveBuffers); } /* TODO - nReusedRXBuffers per queue or per context ?*/ @@ -258,7 +258,7 @@ void CParaNdisRX::ReuseReceiveBufferNoLock(pRxNetDescriptor pBuffersDescriptor) /* TODO - NetMaxReceiveBuffers per queue or per context ?*/ DPrintf(0, "FAILED TO REUSE THE BUFFER!!!!\n"); ParaNdis_FreeRxBufferDescriptor(m_Context, pBuffersDescriptor); - m_Context->NetMaxReceiveBuffers--; + m_NetMaxReceiveBuffers--; } } @@ -538,7 +538,7 @@ void CParaNdisRX::PopulateQueue() /* TODO - NetMaxReceiveBuffers should take into account all queues */ DPrintf(0, "FAILED TO REUSE THE BUFFER!!!!\n"); ParaNdis_FreeRxBufferDescriptor(m_Context, pBufferDescriptor); - m_Context->NetMaxReceiveBuffers--; + m_NetMaxReceiveBuffers--; } } m_Reinsert = true; diff --git a/NetKVM/Common/ndis56common.h b/NetKVM/Common/ndis56common.h index 75420c83e..4dc9f6451 100755 --- a/NetKVM/Common/ndis56common.h +++ b/NetKVM/Common/ndis56common.h @@ -546,7 +546,7 @@ struct _PARANDIS_ADAPTER : public CNdisAllocatable<_PARANDIS_ADAPTER, 'DCTX'> /* initial number of free Tx descriptor(from cfg) - max number of available Tx descriptors */ UINT maxFreeTxDescriptors = 0; /* total of Rx buffer in turnaround */ - UINT NetMaxReceiveBuffers = 0; + UINT maxRxBufferPerQueue = 0; UINT nPnpEventIndex = 0; NDIS_DEVICE_PNP_EVENT PnpEvents[16] = {}; tOffloadSettings Offload = {}; diff --git a/NetKVM/wlh/ParaNdis6_Oid.cpp b/NetKVM/wlh/ParaNdis6_Oid.cpp index 676d8e070..7912bfbd9 100755 --- a/NetKVM/wlh/ParaNdis6_Oid.cpp +++ b/NetKVM/wlh/ParaNdis6_Oid.cpp @@ -499,7 +499,7 @@ static NDIS_STATUS ParaNdis_OidQuery(PARANDIS_ADAPTER *pContext, tOidDesc *pOid) pInfo = &u.WmiConfig; ulSize = sizeof(u.WmiConfig); u.WmiConfig.NumOfQueues = pContext->nPathBundles; - u.WmiConfig.RxQueueSize = pContext->NetMaxReceiveBuffers; + u.WmiConfig.RxQueueSize = pContext->maxRxBufferPerQueue; u.WmiConfig.TxQueueSize = pContext->pPathBundles[0].txPath.GetActualQueueSize(); u.WmiConfig.RscEnabledv4 = pContext->ReportedOffloadConfiguration.Rsc.IPv4.Enabled; u.WmiConfig.RscEnabledv6 = pContext->ReportedOffloadConfiguration.Rsc.IPv6.Enabled; From 9c15277aa6ac0cb44d6d40279671d00443e6520e Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Fri, 29 Nov 2024 19:01:28 +0200 Subject: [PATCH 3/9] netkvm: fix BSOD if some queue fails to renew https://issues.redhat.com/browse/RHEL-68725 https://github.com/virtio-win/kvm-guest-drivers-windows/issues/1197 In such case we do not have any other option than request to unload the driver. This can happen in 2 flows: power up (during fast startup) or reset, both with driver verifier that simulates low resources. In reset flow we need to request driver unload explicitly. Power up flow works in SET_POWER, so it will do it automatically. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-AbstractPath.h | 3 ++- NetKVM/Common/ParaNdis_Common.cpp | 13 +++++++++---- NetKVM/wlh/ParaNdis6_Driver.cpp | 11 +++++++++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/NetKVM/Common/ParaNdis-AbstractPath.h b/NetKVM/Common/ParaNdis-AbstractPath.h index 9d2bb3289..1bd90d409 100644 --- a/NetKVM/Common/ParaNdis-AbstractPath.h +++ b/NetKVM/Common/ParaNdis-AbstractPath.h @@ -49,9 +49,10 @@ class CParaNdisAbstractPath m_pVirtQueue->EnableInterrupts(); } - void Renew() + bool Renew() { m_pVirtQueue->Renew(); + return m_pVirtQueue->IsValid(); } ULONG getCPUIndex(); diff --git a/NetKVM/Common/ParaNdis_Common.cpp b/NetKVM/Common/ParaNdis_Common.cpp index 40359acd7..9096fafb0 100755 --- a/NetKVM/Common/ParaNdis_Common.cpp +++ b/NetKVM/Common/ParaNdis_Common.cpp @@ -2088,7 +2088,7 @@ ParaNdis_UpdateMAC(PARANDIS_ADAPTER *pContext) NDIS_STATUS ParaNdis_PowerOn(PARANDIS_ADAPTER *pContext) { UINT i; - + bool bRenewed = true; DEBUG_ENTRY(0); ParaNdis_DebugHistory(pContext, _etagHistoryLogOperation::hopPowerOn, NULL, 1, 0, 0); @@ -2109,12 +2109,17 @@ NDIS_STATUS ParaNdis_PowerOn(PARANDIS_ADAPTER *pContext) for (i = 0; i < pContext->nPathBundles; i++) { - pContext->pPathBundles[i].txPath.Renew(); - pContext->pPathBundles[i].rxPath.Renew(); + bRenewed = bRenewed && pContext->pPathBundles[i].txPath.Renew(); + bRenewed = bRenewed && pContext->pPathBundles[i].rxPath.Renew(); } if (pContext->bCXPathCreated) { - pContext->CXPath.Renew(); + bRenewed = bRenewed && pContext->CXPath.Renew(); + } + + if (!bRenewed) { + DPrintf(0, "[%s] one or more queues failed to renew\n", __FUNCTION__); + return NDIS_STATUS_RESOURCES; } ParaNdis_RestoreDeviceConfigurationAfterReset(pContext); diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 849eb661b..141b9503c 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -553,12 +553,19 @@ static NDIS_STATUS ParaNdis6_Reset( NDIS_HANDLE miniportAdapterContext, PBOOLEAN pAddressingReset) { + NDIS_STATUS status; PARANDIS_ADAPTER *pContext = (PARANDIS_ADAPTER *)miniportAdapterContext; DEBUG_ENTRY(0); ParaNdis_PowerOff(pContext); - ParaNdis_PowerOn(pContext); + status = ParaNdis_PowerOn(pContext); *pAddressingReset = FALSE; - return NDIS_STATUS_SUCCESS; + // if ParaNdis_PowerOn fails, just returning error + // does not help, so request unload + if (!NT_SUCCESS(status)) { + DPrintf(0, "[%s] requesting removal\n", __FUNCTION__); + NdisMRemoveMiniport(pContext->MiniportHandle); + } + return status; } /*************************************************** From 0489cdbdf963f9c1b94de1642116f063cf5c46ad Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Fri, 29 Nov 2024 19:10:12 +0200 Subject: [PATCH 4/9] netkvm: fix format of printouts in case of error Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis_Common.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/NetKVM/Common/ParaNdis_Common.cpp b/NetKVM/Common/ParaNdis_Common.cpp index 9096fafb0..d7a898f89 100755 --- a/NetKVM/Common/ParaNdis_Common.cpp +++ b/NetKVM/Common/ParaNdis_Common.cpp @@ -1313,25 +1313,25 @@ NDIS_STATUS ParaNdis_FinishInitialization(PARANDIS_ADAPTER *pContext) DEBUG_ENTRY(0); status = ParaNdis_FinishSpecificInitialization(pContext); - DPrintf(0, "[%s] ParaNdis_FinishSpecificInitialization passed, status = %d\n", __FUNCTION__, status); + DPrintf(0, "[%s] ParaNdis_FinishSpecificInitialization passed, status = %X\n", __FUNCTION__, status); if (status == NDIS_STATUS_SUCCESS) { status = ParaNdis_VirtIONetInit(pContext); - DPrintf(0, "[%s] ParaNdis_VirtIONetInit passed, status = %d\n", __FUNCTION__, status); + DPrintf(0, "[%s] ParaNdis_VirtIONetInit passed, status = %X\n", __FUNCTION__, status); } if (status == NDIS_STATUS_SUCCESS && pContext->bUsingMSIX) { status = ParaNdis_ConfigureMSIXVectors(pContext); - DPrintf(0, "[%s] ParaNdis_ConfigureMSIXVectors passed, status = %d\n", __FUNCTION__, status); + DPrintf(0, "[%s] ParaNdis_ConfigureMSIXVectors passed, status = %X\n", __FUNCTION__, status); } if (status == NDIS_STATUS_SUCCESS) { status = SetupDPCTarget(pContext); - DPrintf(0, "[%s] SetupDPCTarget passed, status = %d\n", __FUNCTION__, status); + DPrintf(0, "[%s] SetupDPCTarget passed, status = %X\n", __FUNCTION__, status); } if (status == NDIS_STATUS_SUCCESS && pContext->bPollModeTry && From 7bdcc9bbc1a6070e9a8c3f518068a77042f9713f Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 30 Nov 2024 10:54:31 +0200 Subject: [PATCH 5/9] netkvm: pass miniport context to constructor Read-write lock is created with MiniportContext=NULL because this happens in the constructor when this field in adapter context is not populated yet. Pass miniport context as parameter to the constructor of adapter object and propagate it to the RSSParameters member. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis-RSS.h | 5 +---- NetKVM/Common/ndis56common.h | 8 +++++++- NetKVM/wlh/ParaNdis6_Driver.cpp | 3 +-- NetKVM/wlh/ParaNdis6_RSS.cpp | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/NetKVM/Common/ParaNdis-RSS.h b/NetKVM/Common/ParaNdis-RSS.h index a43f5c117..81f273dfe 100644 --- a/NetKVM/Common/ParaNdis-RSS.h +++ b/NetKVM/Common/ParaNdis-RSS.h @@ -46,7 +46,7 @@ typedef struct _tagPARANDIS_SCALING_SETTINGS class PARANDIS_RSS_PARAMS { public: - PARANDIS_RSS_PARAMS(PARANDIS_ADAPTER *pContext); + PARANDIS_RSS_PARAMS(NDIS_HANDLE MiniportHandle); ~PARANDIS_RSS_PARAMS(); bool FailedInitialization; CCHAR ReceiveQueuesNumber; @@ -61,9 +61,6 @@ class PARANDIS_RSS_PARAMS PARANDIS_SCALING_SETTINGS ActiveRSSScalingSettings = {}; mutable CNdisRWLock rwLock; - -private: - PARANDIS_ADAPTER *m_pContext; }; typedef PARANDIS_RSS_PARAMS *PPARANDIS_RSS_PARAMS; diff --git a/NetKVM/Common/ndis56common.h b/NetKVM/Common/ndis56common.h index 4dc9f6451..9bede7393 100755 --- a/NetKVM/Common/ndis56common.h +++ b/NetKVM/Common/ndis56common.h @@ -449,7 +449,13 @@ struct _tagRxNetDescriptor { struct _PARANDIS_ADAPTER : public CNdisAllocatable<_PARANDIS_ADAPTER, 'DCTX'> { - _PARANDIS_ADAPTER() : guestAnnouncePackets(this), CXPath(this), RSSParameters(this) {} + _PARANDIS_ADAPTER(NDIS_HANDLE Handle) : + MiniportHandle(Handle), + guestAnnouncePackets(this), + CXPath(this), + RSSParameters(Handle) + { + } ~_PARANDIS_ADAPTER(); NDIS_HANDLE DriverHandle = NULL; NDIS_HANDLE MiniportHandle = NULL; diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 141b9503c..06036d55c 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -172,7 +172,7 @@ static NDIS_STATUS ParaNdis6_Initialize( UNREFERENCED_PARAMETER(miniportDriverContext); DEBUG_ENTRY(0); - pContext = new (miniportAdapterHandle) PARANDIS_ADAPTER; + pContext = new (miniportAdapterHandle) PARANDIS_ADAPTER(miniportAdapterHandle); if (!pContext) { @@ -188,7 +188,6 @@ static NDIS_STATUS ParaNdis6_Initialize( /* set mandatory fields which Common use */ pContext->ulUniqueID = NdisInterlockedIncrement(&gID); pContext->DriverHandle = DriverHandle; - pContext->MiniportHandle = miniportAdapterHandle; pContext->m_StateMachine.RegisterFlow(pContext->m_RxStateMachine); pContext->m_StateMachine.RegisterFlow(pContext->m_CxStateMachine); diff --git a/NetKVM/wlh/ParaNdis6_RSS.cpp b/NetKVM/wlh/ParaNdis6_RSS.cpp index 848864e01..6268158f2 100644 --- a/NetKVM/wlh/ParaNdis6_RSS.cpp +++ b/NetKVM/wlh/ParaNdis6_RSS.cpp @@ -1075,9 +1075,9 @@ void ParaNdis6_EnableDeviceRssSupport(PARANDIS_ADAPTER *pContext, BOOLEAN b) } } -PARANDIS_RSS_PARAMS::PARANDIS_RSS_PARAMS(PARANDIS_ADAPTER *pContext) : m_pContext(pContext) +PARANDIS_RSS_PARAMS::PARANDIS_RSS_PARAMS(NDIS_HANDLE MiniportHandle) { - FailedInitialization = !rwLock.Create(pContext->MiniportHandle); + FailedInitialization = !rwLock.Create(MiniportHandle); } PARANDIS_RSS_PARAMS::~PARANDIS_RSS_PARAMS() From b5e6da82fed0335dbc3cfbfd14289bfca2bff764 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 30 Nov 2024 11:02:03 +0200 Subject: [PATCH 6/9] netkvm: move RegisterFlow calls to constructor Signed-off-by: Yuri Benditovich --- NetKVM/Common/ndis56common.h | 2 ++ NetKVM/wlh/ParaNdis6_Driver.cpp | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/NetKVM/Common/ndis56common.h b/NetKVM/Common/ndis56common.h index 9bede7393..37407d157 100755 --- a/NetKVM/Common/ndis56common.h +++ b/NetKVM/Common/ndis56common.h @@ -455,6 +455,8 @@ struct _PARANDIS_ADAPTER : public CNdisAllocatable<_PARANDIS_ADAPTER, 'DCTX'> CXPath(this), RSSParameters(Handle) { + m_StateMachine.RegisterFlow(m_RxStateMachine); + m_StateMachine.RegisterFlow(m_CxStateMachine); } ~_PARANDIS_ADAPTER(); NDIS_HANDLE DriverHandle = NULL; diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 06036d55c..889e94be7 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -189,9 +189,6 @@ static NDIS_STATUS ParaNdis6_Initialize( pContext->ulUniqueID = NdisInterlockedIncrement(&gID); pContext->DriverHandle = DriverHandle; - pContext->m_StateMachine.RegisterFlow(pContext->m_RxStateMachine); - pContext->m_StateMachine.RegisterFlow(pContext->m_CxStateMachine); - NdisZeroMemory(&miniportAttributes, sizeof(miniportAttributes)); miniportAttributes.RegistrationAttributes.Header.Type = NDIS_OBJECT_TYPE_MINIPORT_ADAPTER_REGISTRATION_ATTRIBUTES; miniportAttributes.RegistrationAttributes.Header.Revision = NDIS_MINIPORT_ADAPTER_REGISTRATION_ATTRIBUTES_REVISION_1; From cd63c960ea251050c6283ca90ee670d989003636 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 30 Nov 2024 11:03:33 +0200 Subject: [PATCH 7/9] netkvm: ensure the adapter object is always deleted https://issues.redhat.com/browse/RHEL-68725 Simplify failure flow, remove redundant things from it and ensure the pContext is always deleted. It's destructor does complete cleanup, including interrupt deregistration. This fixes BSOD under driver verifier with low resource simulation NDIS_BUGCHECK_INIT_FAILED_WITHOUT_INTERRUPT_DEREGISTER. Signed-off-by: Yuri Benditovich --- NetKVM/wlh/ParaNdis6_Driver.cpp | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 889e94be7..8af162e72 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -307,26 +307,12 @@ static NDIS_STATUS ParaNdis6_Initialize( } } - if (pContext && status != NDIS_STATUS_SUCCESS && status != NDIS_STATUS_PENDING) - { - pContext->m_StateMachine.UnregisterFlow(pContext->m_RxStateMachine); - pContext->m_StateMachine.UnregisterFlow(pContext->m_CxStateMachine); - pContext->m_StateMachine.NotifyHalted(); - - pContext->Destroy(pContext, pContext->MiniportHandle); - pContext = NULL; - } - - if (pContext && status == NDIS_STATUS_SUCCESS) + if (status == NDIS_STATUS_SUCCESS) { status = ParaNdis_FinishInitialization(pContext); - if (status != NDIS_STATUS_SUCCESS) - { - pContext->Destroy(pContext, pContext->MiniportHandle); - pContext = NULL; - } } - if (pContext && status == NDIS_STATUS_SUCCESS) + + if (status == NDIS_STATUS_SUCCESS) { if (NDIS_STATUS_SUCCESS == ParaNdis6_GetRegistrationOffloadInfo(pContext, @@ -338,7 +324,7 @@ static NDIS_STATUS ParaNdis6_Initialize( } } - if (pContext && status == NDIS_STATUS_SUCCESS) + if (status == NDIS_STATUS_SUCCESS) { pContext->m_StateMachine.NotifyInitialized(pContext); ParaNdis_DebugRegisterMiniport(pContext, TRUE); @@ -346,6 +332,10 @@ static NDIS_STATUS ParaNdis6_Initialize( } else { + if (pContext) + { + pContext->Destroy(pContext, pContext->MiniportHandle); + } // In rare case of initialization failure we need to unregister the protocol. // Use dummy adapter context ParaNdis_ProtocolUnregisterAdapter(); From 73c4ec5182199a1dc120f58cef76e947b3cbe6fc Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sat, 30 Nov 2024 13:36:52 +0200 Subject: [PATCH 8/9] netkvm: fail init if failed to open NIC configuration Under driver verifier with low resources simulation the attempt to access NIC configuration may fail, important fields of the context are not initialized correctly causing driver verifier BSOD on attempt to allocate zero bytes from memory. Fail initialization if the NIC configuration is not accessible. Signed-off-by: Yuri Benditovich --- NetKVM/Common/ParaNdis_Common.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/NetKVM/Common/ParaNdis_Common.cpp b/NetKVM/Common/ParaNdis_Common.cpp index d7a898f89..c23b062db 100755 --- a/NetKVM/Common/ParaNdis_Common.cpp +++ b/NetKVM/Common/ParaNdis_Common.cpp @@ -237,9 +237,10 @@ Loads NIC parameters from adapter registry key context PUCHAR *ppNewMACAddress - pointer to hold MAC address if configured from host ***********************************************************/ -static void ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddress) +static bool ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddress) { NDIS_HANDLE cfg; + bool ret = false; tConfigurationEntries *pConfiguration = (tConfigurationEntries *) ParaNdis_AllocateMemory(pContext, sizeof(tConfigurationEntries)); if (pConfiguration) { @@ -247,6 +248,7 @@ static void ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddre cfg = ParaNdis_OpenNICConfiguration(pContext); if (cfg) { + ret = true; GetConfigurationEntry(cfg, &pConfiguration->PhysicalMediaType); GetConfigurationEntry(cfg, &pConfiguration->isLogEnabled); GetConfigurationEntry(cfg, &pConfiguration->debugLevel); @@ -384,6 +386,7 @@ static void ReadNicConfiguration(PARANDIS_ADAPTER *pContext, PUCHAR pNewMACAddre } NdisFreeMemory(pConfiguration, 0, 0); } + return ret; } void ParaNdis_ResetOffloadSettings(PARANDIS_ADAPTER *pContext, tOffloadSettingsFlags *pDest, PULONG from) @@ -799,7 +802,10 @@ NDIS_STATUS ParaNdis_InitializeContext( return NDIS_STATUS_RESOURCES; } - ReadNicConfiguration(pContext, CurrentMAC); + if (!ReadNicConfiguration(pContext, CurrentMAC)) + { + return NDIS_STATUS_RESOURCES; + } pContext->fCurrentLinkState = MediaConnectStateUnknown; From 4a32d3007276aa41669a929fa5e567cd36b6dab3 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Sun, 1 Dec 2024 16:07:22 +0200 Subject: [PATCH 9/9] netkvm: remove unused member 'DriverHandle' Signed-off-by: Yuri Benditovich --- NetKVM/Common/ndis56common.h | 1 - NetKVM/wlh/ParaNdis6_Driver.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/NetKVM/Common/ndis56common.h b/NetKVM/Common/ndis56common.h index 37407d157..5b9066ac0 100755 --- a/NetKVM/Common/ndis56common.h +++ b/NetKVM/Common/ndis56common.h @@ -459,7 +459,6 @@ struct _PARANDIS_ADAPTER : public CNdisAllocatable<_PARANDIS_ADAPTER, 'DCTX'> m_StateMachine.RegisterFlow(m_CxStateMachine); } ~_PARANDIS_ADAPTER(); - NDIS_HANDLE DriverHandle = NULL; NDIS_HANDLE MiniportHandle = NULL; NDIS_HANDLE InterruptHandle = NULL; NDIS_HANDLE BufferListsPool = NULL; diff --git a/NetKVM/wlh/ParaNdis6_Driver.cpp b/NetKVM/wlh/ParaNdis6_Driver.cpp index 8af162e72..13b132535 100755 --- a/NetKVM/wlh/ParaNdis6_Driver.cpp +++ b/NetKVM/wlh/ParaNdis6_Driver.cpp @@ -187,7 +187,6 @@ static NDIS_STATUS ParaNdis6_Initialize( /* set mandatory fields which Common use */ pContext->ulUniqueID = NdisInterlockedIncrement(&gID); - pContext->DriverHandle = DriverHandle; NdisZeroMemory(&miniportAttributes, sizeof(miniportAttributes)); miniportAttributes.RegistrationAttributes.Header.Type = NDIS_OBJECT_TYPE_MINIPORT_ADAPTER_REGISTRATION_ATTRIBUTES;