linux-kernel - Re: [PATCH net] bnxt_en: Fix NULL pointer crash in bnxt_ptp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <nejlqwxc4ekfhmpodjm63cfob4o5uf2z7qukk3daofykegnwvs@sksxy4lmxrnd>
Date: Fri, 2 Jan 2026 07:43:07 -0800
From: Breno Leitao <leitao@...ian.org>
To: Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc: Michael Chan <michael.chan@...adcom.com>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, 
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>, Vadim Fedorenko <vadim.fedorenko@...ux.dev>, 
	Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kernel-team@...a.com, stable@...r.kernel.org
Subject: Re: [PATCH net] bnxt_en: Fix NULL pointer crash in bnxt_ptp_enable
 during error cleanup

Hello Pavan,

On Wed, Dec 31, 2025 at 09:30:57PM +0530, Pavan Chebbi wrote:
> On Wed, Dec 31, 2025 at 6:35 PM Breno Leitao <leitao@...ian.org> wrote:
> > Fix this by checking if bp->hwrm_dma_pool is NULL at the start of
> > bnxt_ptp_enable(). During error/cleanup paths when HWRM resources have
> > been freed, return success without attempting to send commands since the
> > hardware is being torn down anyway.
> >
> > During normal operation, the DMA pool is always valid so PTP
> > functionality is unaffected.
> >
> > Signed-off-by: Breno Leitao <leitao@...ian.org>
> > Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
> > Cc: stable@...r.kernel.org
> > ---
> >  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> > index a8a74f07bb54..a749bbfa398e 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> > @@ -482,6 +482,13 @@ static int bnxt_ptp_enable(struct ptp_clock_info *ptp_info,
> >         int pin_id;
> >         int rc;
> >
> > +       /* Return success if HWRM resources are not available.
> > +        * This can happen during error/cleanup paths when DMA pool has been
> > +        * freed.
> > +        */
> > +       if (!bp->hwrm_dma_pool)
> 
> Thanks for the fix. While it's valid, just that to me, this check here
> looks a bit odd.
> Why not call bnxt_ptp_clear() before bnxt_free_hwrm_resources() in the
> unwind path?

I thought about it, but, I didn't understand all the implication of
changing the unwind order. 

Anyway, I've have tested the current patch and it worked fine. Do you
think we should move kfree(bp->ptp_cfg) closer to bnxt_ptp_clear()?

Thanks for the review,
--breno


commit d07c08889f75966d6829b93304de5030cf4e66aa
Author: Breno Leitao <leitao@...ian.org>
Date:   Wed Dec 31 04:00:57 2025 -0800

    bnxt_en: Fix NULL pointer crash in bnxt_ptp_enable during error cleanup
    
    When bnxt_init_one() fails during initialization (e.g.,
    bnxt_init_int_mode returns -ENODEV), the error path calls
    bnxt_free_hwrm_resources() which destroys the DMA pool and sets
    bp->hwrm_dma_pool to NULL. Subsequently, bnxt_ptp_clear() is called,
    which invokes ptp_clock_unregister().
    
    Since commit a60fc3294a37 ("ptp: rework ptp_clock_unregister() to
    disable events"), ptp_clock_unregister() now calls
    ptp_disable_all_events(), which in turn invokes the driver's .enable()
    callback (bnxt_ptp_enable()) to disable PTP events before completing the
    unregistration.
    
    bnxt_ptp_enable() attempts to send HWRM commands via bnxt_ptp_cfg_pin()
    and bnxt_ptp_cfg_event(), both of which call hwrm_req_init(). This
    function tries to allocate from bp->hwrm_dma_pool, causing a NULL
    pointer dereference:
    
      bnxt_en 0000:01:00.0 (unnamed net_device) (uninitialized): bnxt_init_int_mode err: ffffffed
      KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
      Call Trace:
       __hwrm_req_init (drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c:72)
       bnxt_ptp_enable (drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:323 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:517)
       ptp_disable_all_events (drivers/ptp/ptp_chardev.c:66)
       ptp_clock_unregister (drivers/ptp/ptp_clock.c:518)
       bnxt_ptp_clear (drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:1134)
       bnxt_init_one (drivers/net/ethernet/broadcom/bnxt/bnxt.c:16889)
    
    Lines are against commit f8f9c1f4d0c7 ("Linux 6.19-rc3")
    
    Fix this by clearing and unregistering ptp (bnxt_ptp_clear()) before
    freeing HWRM resources.
    
    Suggested-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
    Signed-off-by: Breno Leitao <leitao@...ian.org>
    Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
    Cc: stable@...r.kernel.org

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d17d0ea89c36..68fc9977b375 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16882,10 +16882,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 init_err_pci_clean:
 	bnxt_hwrm_func_drv_unrgtr(bp);
+	bnxt_ptp_clear(bp);
 	bnxt_free_hwrm_resources(bp);
 	bnxt_hwmon_uninit(bp);
 	bnxt_ethtool_free(bp);
-	bnxt_ptp_clear(bp);
 	kfree(bp->ptp_cfg);
 	bp->ptp_cfg = NULL;
 	kfree(bp->fw_health);