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

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 <[email protected]>
> > > Reviewed-by: Breno Leitao <[email protected]>
> > 
> > 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 <[email protected]>