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

[PATCH net v2] 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.

Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
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 v2] net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

[Breno Leitao]

On Mon, Aug 03, 2026 at 03:20:21PM +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.
> 
> 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>

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

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling

This patch avoids dereferencing wx->msix_entry in NGBE_INTR_MISC() when the
device is not running in MSI-X mode, by testing pdev->msix_enabled in
ngbe_irq_enable() and falling back to a hard-coded mask value otherwise.

> 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));
>  }

[Severity: Medium]
Is BIT(0) the right mask bit for the misc cause in the non-MSI-X case?

In the MSI/INTx fallback, wx_set_interrupt_capability() sets
wx->num_q_vectors = 1, and ngbe_up() -> wx_configure_vectors() still
programs the misc IVAR:

drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_configure_vectors() {
	...
	wx_set_ivar(wx, -1, 0, v_idx);
}

with v_idx == wx->num_q_vectors == 1.  wx_set_ivar() only forces the
other-causes vector to 0 when WX_FLAG_IRQ_VECTOR_SHARED is set, which
happens for wx_mac_em with num_vfs == 7 in __wx_enable_sriov():

	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
		msix_vector = 0;

So in plain MSI/INTx the misc cause sits at bit 1, and bit 0 is queue
vector 0.  Does this branch then clear the wrong bit in PX_IMC, leaving
the misc cause masked while unmasking the queue cause?

The queue cause looks like it is meant to stay masked here, per
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);

and wx_poll() is the place that re-arms it after NAPI completes:

	wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));

Can unmasking BIT(0) from the hard IRQ handler let the device re-assert
the (possibly shared) INTx/MSI line while NAPI is still scheduled,
leading to repeated hard-IRQ entries and, on the eicr == 0 path, to
IRQ_NONE returns followed by a full NGBE_INTR_ALL unmask?

Would BIT(wx->num_q_vectors) be the value that preserves the behaviour
prior to commit 4174c0c331a2, where NGBE_INTR_MISC(A) expanded to
BIT((A)->num_q_vectors)?  The sibling driver still uses that form
unconditionally, including on its non-MSI-X path:

drivers/net/ethernet/wangxun/txgbe/txgbe_type.h:
	#define TXGBE_INTR_MISC(A)	BIT((A)->num_q_vectors)

One more note on the changelog: it says the fix is to check
pdev->msix_enabled "to determine the correct vector index", but it does
not mention that the mask value written in non-MSI-X mode changes from
BIT(1) to BIT(0).  Could the changelog spell out the intended vector
index for the MSI/INTx case?
-- 
pw-bot: cr

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

[Jiawen Wu]

On Thu, Aug 6, 2026 11:32 PM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: ngbe: fix NULL pointer dereference in non-MSI-X interrupt enabling
> 
> This patch avoids dereferencing wx->msix_entry in NGBE_INTR_MISC() when the
> device is not running in MSI-X mode, by testing pdev->msix_enabled in
> ngbe_irq_enable() and falling back to a hard-coded mask value otherwise.
> 
> > 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));
> >  }
> 
> [Severity: Medium]
> Is BIT(0) the right mask bit for the misc cause in the non-MSI-X case?

In the non-MSI-X case, the MISC and queue interrupt share the vector 0.

> In the MSI/INTx fallback, wx_set_interrupt_capability() sets
> wx->num_q_vectors = 1, and ngbe_up() -> wx_configure_vectors() still
> programs the misc IVAR:
> 
> drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_configure_vectors() {
> 	...
> 	wx_set_ivar(wx, -1, 0, v_idx);
> }

The register WX_PX_MISC_IVAR is only valid in the MSI-X case.

> with v_idx == wx->num_q_vectors == 1.  wx_set_ivar() only forces the
> other-causes vector to 0 when WX_FLAG_IRQ_VECTOR_SHARED is set, which
> happens for wx_mac_em with num_vfs == 7 in __wx_enable_sriov():
> 
> 	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
> 		msix_vector = 0;
> 
> So in plain MSI/INTx the misc cause sits at bit 1, and bit 0 is queue
> vector 0.  Does this branch then clear the wrong bit in PX_IMC, leaving
> the misc cause masked while unmasking the queue cause?
> 
> The queue cause looks like it is meant to stay masked here, per
> 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);
> 
> and wx_poll() is the place that re-arms it after NAPI completes:
> 
> 	wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));
> 
> Can unmasking BIT(0) from the hard IRQ handler let the device re-assert
> the (possibly shared) INTx/MSI line while NAPI is still scheduled,
> leading to repeated hard-IRQ entries and, on the eicr == 0 path, to
> IRQ_NONE returns followed by a full NGBE_INTR_ALL unmask?
> 
> Would BIT(wx->num_q_vectors) be the value that preserves the behaviour
> prior to commit 4174c0c331a2, where NGBE_INTR_MISC(A) expanded to
> BIT((A)->num_q_vectors)?  The sibling driver still uses that form
> unconditionally, including on its non-MSI-X path:
> 
> drivers/net/ethernet/wangxun/txgbe/txgbe_type.h:
> 	#define TXGBE_INTR_MISC(A)	BIT((A)->num_q_vectors)
> 
> One more note on the changelog: it says the fix is to check
> pdev->msix_enabled "to determine the correct vector index", but it does
> not mention that the mask value written in non-MSI-X mode changes from
> BIT(1) to BIT(0).  Could the changelog spell out the intended vector
> index for the MSI/INTx case?

I'll add a log to describe this in passing fix.