Subject: Re: [PATCH net v2] bnxt_en: Fix NULL pointer crash in bnxt_ptp_enable during error cleanup

From: "Russell King (Oracle)" <[email-masked]>
Date: Mon, 5 Jan 2026 18:03:31 +0000

On Mon, Jan 05, 2026 at 09:40:03AM -0800, Michael Chan wrote:
> On Mon, Jan 5, 2026 at 7:51 AM Pavan Chebbi <[email-masked]> wrote:
> >
> > On Mon, Jan 5, 2026 at 6:59 PM Russell King (Oracle)
> > <[email-masked]> wrote:
> > >
> 
> > > Second, __bnxt_hwrm_ptp_qcfg() calls bnxt_ptp_clear() if
> > > bp->hwrm_spec_code < 0x10801 || !BNXT_CHIP_P5_PLUS(bp) is true or
> > > hwrm_req_init() fails. Is it really possible that we have the PTP
> > > clock registered when PTP isn't supported?
> >
> > Right, this check may not make much sense because we call
> > __bnxt_hwrm_ptp_qcfg() only after we know PTP is supported.
> > Michael may tell better but I think we could improve by removing that check.
> >
> 
> Some older FW may advertise support for PTP using an older scheme that
> the driver does not support.  The FW running on an older class of
> chips may also advertise support for PTP and it's also not supported
> by the driver.  In the former case, if FW is downgraded, the test may
> become true.

I'd like to restate my question, as it is the crux of the issue: as
the PTP clock remains registered during the firmware change,
userspace can interact with that device in every way possible.

If the firmware is in the process of being changed, and e.g.
bnxt_ptp_enable() were to be called by way of userspace interacting
with the PTP clock, we have already established that bnxt_ptp_enable()
will talk to the firmware - but what happens if bnxt_ptp_enable()
attempts to while the firmware is being changed? Is this safe?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!