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] [day] [month] [year] [list]

Message-ID: <CALs4sv2qQuL0trq3ZB6SczPK5BmFMF6p2Ki-3q+4Xqc_qzauoQ@mail.gmail.com>
Date: Wed, 31 Dec 2025 21:30:57 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Breno Leitao <leitao@...ian.org>
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

On Wed, Dec 31, 2025 at 6:35 PM Breno Leitao <leitao@...ian.org> wrote:
>
> 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 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?

> +               return 0;
> +
>         switch (rq->type) {
>         case PTP_CLK_REQ_EXTTS:
>                 /* Configure an External PPS IN */
>
> ---
> base-commit: 80380f6ce46f38a0d1200caade2f03de4b6c1d27
> change-id: 20251231-bnxt-c54d317d8bfe
>
> Best regards,
> --
> Breno Leitao <leitao@...ian.org>
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5469 bytes)