[PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[Jiawen Wu]

In non-MSI-X mode (such as legacy INTx or single MSI), wx->msix_entry is
not allocated or initialized. Calling NGBE_INTR_MISC(wx) dereferences
wx->msix_entry->entry, leading to a NULL pointer dereference crash.

This issue was introduced by fixing the IRQ vector when the number of
VFs is 7. Fix the issue by explicitly checking `pdev->msix_enabled` to
determine the correct vector index.

Additionally, as a side fix, set the interrupt mask to BIT(0) for the
non-MSI-X fallback. In MSI/INTx mode, the MISC and queue interrupts
share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
when MSI-X is disabled.

Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
---
v2 -> v3: Detail the commit log.
v1 -> v2: Use pdev->msix_enabled and reserve NGBE_INTR_MISC.
---
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index a16221995909..cb653861aad8 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -180,8 +180,10 @@ static void ngbe_irq_enable(struct wx *wx, bool queues)
 	/* mask interrupt */
 	if (queues)
 		wx_intr_enable(wx, NGBE_INTR_ALL);
-	else
+	else if (wx->pdev->msix_enabled)
 		wx_intr_enable(wx, NGBE_INTR_MISC(wx));
+	else
+		wx_intr_enable(wx, BIT(0));
 }
 
 /**
-- 
2.51.0

Re: [PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[Simon Horman]

On Fri, Aug 07, 2026 at 02:22:14PM +0800, Jiawen Wu wrote:
> In non-MSI-X mode (such as legacy INTx or single MSI), wx->msix_entry is
> not allocated or initialized. Calling NGBE_INTR_MISC(wx) dereferences
> wx->msix_entry->entry, leading to a NULL pointer dereference crash.
> 
> This issue was introduced by fixing the IRQ vector when the number of
> VFs is 7. Fix the issue by explicitly checking `pdev->msix_enabled` to
> determine the correct vector index.
> 
> Additionally, as a side fix, set the interrupt mask to BIT(0) for the
> non-MSI-X fallback. In MSI/INTx mode, the MISC and queue interrupts
> share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
> MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
> when MSI-X is disabled.
> 
> Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> Reviewed-by: Breno Leitao <leitao@debian.org>

Hi Jiawen,

I know the topic of BIT(0) came up in v2 - the AI-generated review
forwarded by Jakub. And I appreciate you clarifying things in the commit
message. But the AI generated review on netdev-qi [1] raises concerns which
I'd appreciate you looking over.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/B2693E9A8BFAD110%2B20260807062214.410838-1-jiawenwu%40trustnetic.com

Although the review rehashes the concerns raised for v2, which you have
addressed.  There are three related areas of concern which I can't resolve in
my mind.

  1) The question of unmasking IRQ 0,
     and the implication of possible interrupt storms.

  2) Compatibility of the BIT(0) change in this patch with the implementation
     of wx_configure_vectors()

  3) Is there a need for any similar changes to txgbe.

Text from the AI-generated review is as follows:

  Is BIT(0) the right mask here, or is it the mask bit of queue vector 0
  rather than of the misc cause?

  In MSI/INTx mode wx_set_interrupt_capability() sets wx->num_q_vectors = 1 and
  q_vector[0]->v_idx == 0, and the queue vector's mask bit is BIT(v_idx), since
  WX_INTR_Q(i) is BIT((i)) and wx_poll() re-arms the queue with:

  drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_poll() {
	...
	if (likely(napi_complete_done(napi, work_done))) {
		...
		if (!test_bit(WX_STATE_DOWN, wx->state))
			wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));
	}
	...
  }

  So does the new fallback unmask the queue vector on a path whose callers
  describe it as "no lsc, no queues"?

  The concrete effect appears in ngbe_intr(), where the queue cause is
  auto-masked by hardware on the cause read:

  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c:ngbe_intr() {
	...
	/* would disable interrupts here but it is auto disabled */
	napi_schedule_irqoff(&q_vector->napi);
	if (!test_bit(WX_STATE_DOWN, wx->state))
		ngbe_irq_enable(wx, false);
	...
  }

  With BIT(0) the queue vector is unmasked again immediately while NAPI is
  still scheduled. Does this defeat the NAPI moderation in MSI/INTx mode, so
  the driver takes an interrupt per completion event instead of one per NAPI
  cycle?

  The peer driver on the same libwx register layout uses the opposite value,
  unconditionally, including on its non-MSI-X paths in txgbe_irq.c:

  drivers/net/ethernet/wangxun/txgbe/txgbe_type.h:

  #define TXGBE_INTR_MISC(A)    BIT((A)->num_q_vectors)
  #define TXGBE_INTR_QALL(A)    (TXGBE_INTR_MISC(A) - 1)

  ngbe also used BIT(wx->num_q_vectors) for this path before 4174c0c331a2.
  Would BIT(wx->num_q_vectors) be the value that matches libwx and txgbe here,
  or alternatively programming WX_PX_MISC_IVAR to vector 0 when
  !pdev->msix_enabled so that routing and mask agree?

  Related to that, the changelog says:

      In MSI/INTx mode, the MISC and queue interrupts
      share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
      MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
      when MSI-X is disabled.

  Which code establishes that WX_PX_MISC_IVAR is only valid with MSI-X?
  wx_configure_vectors() writes the misc IVAR unconditionally with
  v_idx == wx->num_q_vectors, and only the following ITR write is gated on
  msix_enabled:

  drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_configure_vectors() {
	...
	wx_set_ivar(wx, -1, 0, v_idx);
	if (pdev->msix_enabled)
		wr32(wx, WX_PX_ITR(v_idx), 1950);
  }

  The only place that forces the misc vector index to 0 is the
  WX_FLAG_IRQ_VECTOR_SHARED path set by __wx_enable_sriov() for
  num_vfs == 7 && mac.type == wx_mac_em, which is itself an MSI-X case. Could
  the changelog be adjusted to match what the code does, or is there hardware
  behaviour behind the claim that is not visible in the driver?

RE: [PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[Jiawen Wu]

On Mon, Aug 10, 2026 11:39 PM, Simon Horman wrote:
> On Fri, Aug 07, 2026 at 02:22:14PM +0800, Jiawen Wu wrote:
> > In non-MSI-X mode (such as legacy INTx or single MSI), wx->msix_entry is
> > not allocated or initialized. Calling NGBE_INTR_MISC(wx) dereferences
> > wx->msix_entry->entry, leading to a NULL pointer dereference crash.
> >
> > This issue was introduced by fixing the IRQ vector when the number of
> > VFs is 7. Fix the issue by explicitly checking `pdev->msix_enabled` to
> > determine the correct vector index.
> >
> > Additionally, as a side fix, set the interrupt mask to BIT(0) for the
> > non-MSI-X fallback. In MSI/INTx mode, the MISC and queue interrupts
> > share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
> > MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
> > when MSI-X is disabled.
> >
> > Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > Reviewed-by: Breno Leitao <leitao@debian.org>
> 
> Hi Jiawen,
> 
> I know the topic of BIT(0) came up in v2 - the AI-generated review
> forwarded by Jakub. And I appreciate you clarifying things in the commit
> message. But the AI generated review on netdev-qi [1] raises concerns which
> I'd appreciate you looking over.
> 
> [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/B2693E9A8BFAD110%2B20260807062214.410838-1-jiawenwu%40trustnetic.com
> 
> Although the review rehashes the concerns raised for v2, which you have
> addressed.  There are three related areas of concern which I can't resolve in
> my mind.
> 
>   1) The question of unmasking IRQ 0,
>      and the implication of possible interrupt storms.
> 
>   2) Compatibility of the BIT(0) change in this patch with the implementation
>      of wx_configure_vectors()
> 
>   3) Is there a need for any similar changes to txgbe.

1) The hardware truth regarding WX_PX_MISC_IVAR and BIT(0):
Sashiko assumes that in non-MSI-X mode, the MISC interrupt cause is mapped to
BIT(1) because wx_configure_vectors() unconditionally writes to the
WX_PX_MISC_IVAR register. 

In reality, on Wangxun hardware, the WX_PX_MISC_IVAR register is *completely
ignored by the hardware* when MSI-X is not enabled. In legacy INTx or single
MSI mode, the hardware forcibly merges all interrupt causes (both Queue and
MISC) into a single bit: BIT(0) of the interrupt cause register. The
configuration in wx_configure_vectors() for non-MSI-X mode is essentially dead
code that has no hardware effect. Therefore, BIT(0) is the only correct mask to
clear.

2) Why it won't cause an interrupt storm:
wx_intr_enable() clears the mask register to *allow* the next interrupt to be
asserted, it does not force-trigger an interrupt.

Since Queue and MISC are physically bound to BIT(0) in this mode, if we do not
unmask BIT(0) here, or mistakenly unmask BIT(1) (which the hardware never uses
in this mode), the MISC interrupt will remain masked indefinitely. This would
lead to lost MISC interrupts. The NAPI polling will still correctly handle the
Rx/Tx cleanups without looping endlessly.

3) Regarding txgbe consistency:
txgbe suffers from the exact same conceptual bug in its non-MSI-X fallback path.
I will be submitting a separate patch to fix txgbe as well.

> 
> Text from the AI-generated review is as follows:
> 
>   Is BIT(0) the right mask here, or is it the mask bit of queue vector 0
>   rather than of the misc cause?
> 
>   In MSI/INTx mode wx_set_interrupt_capability() sets wx->num_q_vectors = 1 and
>   q_vector[0]->v_idx == 0, and the queue vector's mask bit is BIT(v_idx), since
>   WX_INTR_Q(i) is BIT((i)) and wx_poll() re-arms the queue with:
> 
>   drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_poll() {
> 	...
> 	if (likely(napi_complete_done(napi, work_done))) {
> 		...
> 		if (!test_bit(WX_STATE_DOWN, wx->state))
> 			wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));
> 	}
> 	...
>   }
> 
>   So does the new fallback unmask the queue vector on a path whose callers
>   describe it as "no lsc, no queues"?
> 
>   The concrete effect appears in ngbe_intr(), where the queue cause is
>   auto-masked by hardware on the cause read:
> 
>   drivers/net/ethernet/wangxun/ngbe/ngbe_main.c:ngbe_intr() {
> 	...
> 	/* would disable interrupts here but it is auto disabled */
> 	napi_schedule_irqoff(&q_vector->napi);
> 	if (!test_bit(WX_STATE_DOWN, wx->state))
> 		ngbe_irq_enable(wx, false);
> 	...
>   }
> 
>   With BIT(0) the queue vector is unmasked again immediately while NAPI is
>   still scheduled. Does this defeat the NAPI moderation in MSI/INTx mode, so
>   the driver takes an interrupt per completion event instead of one per NAPI
>   cycle?
> 
>   The peer driver on the same libwx register layout uses the opposite value,
>   unconditionally, including on its non-MSI-X paths in txgbe_irq.c:
> 
>   drivers/net/ethernet/wangxun/txgbe/txgbe_type.h:
> 
>   #define TXGBE_INTR_MISC(A)    BIT((A)->num_q_vectors)
>   #define TXGBE_INTR_QALL(A)    (TXGBE_INTR_MISC(A) - 1)
> 
>   ngbe also used BIT(wx->num_q_vectors) for this path before 4174c0c331a2.
>   Would BIT(wx->num_q_vectors) be the value that matches libwx and txgbe here,
>   or alternatively programming WX_PX_MISC_IVAR to vector 0 when
>   !pdev->msix_enabled so that routing and mask agree?
> 
>   Related to that, the changelog says:
> 
>       In MSI/INTx mode, the MISC and queue interrupts
>       share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
>       MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
>       when MSI-X is disabled.
> 
>   Which code establishes that WX_PX_MISC_IVAR is only valid with MSI-X?
>   wx_configure_vectors() writes the misc IVAR unconditionally with
>   v_idx == wx->num_q_vectors, and only the following ITR write is gated on
>   msix_enabled:
> 
>   drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_configure_vectors() {
> 	...
> 	wx_set_ivar(wx, -1, 0, v_idx);
> 	if (pdev->msix_enabled)
> 		wr32(wx, WX_PX_ITR(v_idx), 1950);
>   }
> 
>   The only place that forces the misc vector index to 0 is the
>   WX_FLAG_IRQ_VECTOR_SHARED path set by __wx_enable_sriov() for
>   num_vfs == 7 && mac.type == wx_mac_em, which is itself an MSI-X case. Could
>   the changelog be adjusted to match what the code does, or is there hardware
>   behaviour behind the claim that is not visible in the driver?

Re: [PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[Simon Horman]

On Tue, Aug 11, 2026 at 10:19:28AM +0800, Jiawen Wu wrote:
> On Mon, Aug 10, 2026 11:39 PM, Simon Horman wrote:
> > On Fri, Aug 07, 2026 at 02:22:14PM +0800, Jiawen Wu wrote:
> > > In non-MSI-X mode (such as legacy INTx or single MSI), wx->msix_entry is
> > > not allocated or initialized. Calling NGBE_INTR_MISC(wx) dereferences
> > > wx->msix_entry->entry, leading to a NULL pointer dereference crash.
> > >
> > > This issue was introduced by fixing the IRQ vector when the number of
> > > VFs is 7. Fix the issue by explicitly checking `pdev->msix_enabled` to
> > > determine the correct vector index.
> > >
> > > Additionally, as a side fix, set the interrupt mask to BIT(0) for the
> > > non-MSI-X fallback. In MSI/INTx mode, the MISC and queue interrupts
> > > share vector 0, and the WX_PX_MISC_IVAR register is only valid in the
> > > MSI-X case. Thus, BIT(0) is the correct mask for the miscellaneous cause
> > > when MSI-X is disabled.
> > >
> > > Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
> > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > > Reviewed-by: Breno Leitao <leitao@debian.org>
> > 
> > Hi Jiawen,
> > 
> > I know the topic of BIT(0) came up in v2 - the AI-generated review
> > forwarded by Jakub. And I appreciate you clarifying things in the commit
> > message. But the AI generated review on netdev-qi [1] raises concerns which
> > I'd appreciate you looking over.
> > 
> > [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/B2693E9A8BFAD110%2B20260807062214.410838-1-jiawenwu%40trustnetic.com
> > 
> > Although the review rehashes the concerns raised for v2, which you have
> > addressed.  There are three related areas of concern which I can't resolve in
> > my mind.
> > 
> >   1) The question of unmasking IRQ 0,
> >      and the implication of possible interrupt storms.
> > 
> >   2) Compatibility of the BIT(0) change in this patch with the implementation
> >      of wx_configure_vectors()
> > 
> >   3) Is there a need for any similar changes to txgbe.
> 
> 1) The hardware truth regarding WX_PX_MISC_IVAR and BIT(0):
> Sashiko assumes that in non-MSI-X mode, the MISC interrupt cause is mapped to
> BIT(1) because wx_configure_vectors() unconditionally writes to the
> WX_PX_MISC_IVAR register. 
> 
> In reality, on Wangxun hardware, the WX_PX_MISC_IVAR register is *completely
> ignored by the hardware* when MSI-X is not enabled. In legacy INTx or single
> MSI mode, the hardware forcibly merges all interrupt causes (both Queue and
> MISC) into a single bit: BIT(0) of the interrupt cause register. The
> configuration in wx_configure_vectors() for non-MSI-X mode is essentially dead
> code that has no hardware effect. Therefore, BIT(0) is the only correct mask to
> clear.
> 
> 2) Why it won't cause an interrupt storm:
> wx_intr_enable() clears the mask register to *allow* the next interrupt to be
> asserted, it does not force-trigger an interrupt.
> 
> Since Queue and MISC are physically bound to BIT(0) in this mode, if we do not
> unmask BIT(0) here, or mistakenly unmask BIT(1) (which the hardware never uses
> in this mode), the MISC interrupt will remain masked indefinitely. This would
> lead to lost MISC interrupts. The NAPI polling will still correctly handle the
> Rx/Tx cleanups without looping endlessly.
> 
> 3) Regarding txgbe consistency:
> txgbe suffers from the exact same conceptual bug in its non-MSI-X fallback path.
> I will be submitting a separate patch to fix txgbe as well.

Hi Jiawen,

Thanks for your detailed response. I agree that all is in order here.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri,  7 Aug 2026 14:22:14 +0800 you wrote:
> In non-MSI-X mode (such as legacy INTx or single MSI), wx->msix_entry is
> not allocated or initialized. Calling NGBE_INTR_MISC(wx) dereferences
> wx->msix_entry->entry, leading to a NULL pointer dereference crash.
> 
> This issue was introduced by fixing the IRQ vector when the number of
> VFs is 7. Fix the issue by explicitly checking `pdev->msix_enabled` to
> determine the correct vector index.
> 
> [...]

Here is the summary with links:
  - [net,v3] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling
    https://git.kernel.org/netdev/net/c/5f3a13e0bb5e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html