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

Jiawen Wu <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <[email protected]>
On Fri, Jul 31, 2026 9:51 PM, Simon Horman wrote:
> On Tue, Jul 28, 2026 at 05:35:54PM +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. Since macro NGBE_INTR_MISC is used in only one place and
> > relies on MSI-X allocation, remove it.
> >
> > Fix the issue by explicitly checking WX_FLAG_IRQ_VECTOR_SHARED to
> > determine the correct vector index and using BIT() to convert it into
> > the interrupt mask required by wx_intr_enable().
> >
> > Fixes: 4174c0c331a2 ("net: ngbe: specify IRQ vector when the number of VFs is 7")
> > Signed-off-by: Jiawen Wu <[email protected]>
> > ---
> >  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 4 +++-
> >  drivers/net/ethernet/wangxun/ngbe/ngbe_type.h | 1 -
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > index a16221995909..5d89fc84e8bc 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 if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
> > +		wx_intr_enable(wx, BIT(0));
> 
> Hi Jiawen,
> 
> I am wondering if you could take a look over the following issue
> which is included in the AI-generated review on sashiko.dev [1]
> 
> [1] https://sashiko.dev/#/patchset/21947C51A754F86C%2B20260728093554.9324-1-jiawenwu%40trustnetic.com
> 
>   Does this change introduce a desynchronization between the software state
>   and the hardware configuration if SR-IOV enablement fails?
> 
>   If pci_enable_sriov() fails inside wx_pci_sriov_enable(), the error path
>   clears the WX_FLAG_IRQ_VECTOR_SHARED flag via wx_sriov_clear_data():

Looks like a bad time to clear the flag...
I'll change it to check pdev->msix_enabled but not the flag.

> 
>   wx_pci_sriov_enable() {
>       ...
>       err = pci_enable_sriov(wx->pdev, num_vfs);
>       if (err) {
>           ...
>           goto err_out;
>       }
>       ...
>   err_out:
>       wx_sriov_clear_data(wx);
>       return err;
>   }
> 
>   However, the hardware was already re-initialized with the MISC interrupt
>   mapped to vector 0. Since the hardware configuration isn't reverted, won't
>   subsequent shared interrupts cause the hardware to auto-mask vector 0, which
>   will then never be unmasked here because the flag was cleared? This appears
>   to result in a permanent masking of Queue 0 and MISC events.
> 
> >  	else
> > -		wx_intr_enable(wx, NGBE_INTR_MISC(wx));
> > +		wx_intr_enable(wx, BIT(wx->num_q_vectors));
> >  }
> >
> >  /**
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.