Re: [PATCH v3 net 5/8] eth: bnxt: fix kernel panic in the bnxt_get_queue_stats{rx | tx}

[Somnath Kotur <somnath.kotur@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 3738 bytes --]

On Sun, Mar 9, 2025 at 7:13 PM Taehee Yoo <ap420073@gmail.com> wrote:
>
> When qstats-get operation is executed, callbacks of netdev_stats_ops
> are called. The bnxt_get_queue_stats{rx | tx} collect per-queue stats
> from sw_stats in the rings.
> But {rx | tx | cp}_ring are allocated when the interface is up.
> So, these rings are not allocated when the interface is down.
>
> The qstats-get is allowed even if the interface is down. However,
> the bnxt_get_queue_stats{rx | tx}() accesses cp_ring and tx_ring
> without null check.
> So, it needs to avoid accessing rings if the interface is down.
>
> Reproducer:
>  ip link set $interface down
>  ./cli.py --spec netdev.yaml --dump qstats-get
> OR
>  ip link set $interface down
>  python ./stats.py
>
> Splat looks like:
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 1680fa067 P4D 1680fa067 PUD 16be3b067 PMD 0
>  Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
>  CPU: 0 UID: 0 PID: 1495 Comm: python3 Not tainted 6.14.0-rc4+ #32 5cd0f999d5a15c574ac72b3e4b907341
>  Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021
>  RIP: 0010:bnxt_get_queue_stats_rx+0xf/0x70 [bnxt_en]
>  Code: c6 87 b5 18 00 00 02 eb a2 66 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 01
>  RSP: 0018:ffffabef43cdb7e0 EFLAGS: 00010282
>  RAX: 0000000000000000 RBX: ffffffffc04c8710 RCX: 0000000000000000
>  RDX: ffffabef43cdb858 RSI: 0000000000000000 RDI: ffff8d504e850000
>  RBP: ffff8d506c9f9c00 R08: 0000000000000004 R09: ffff8d506bcd901c
>  R10: 0000000000000015 R11: ffff8d506bcd9000 R12: 0000000000000000
>  R13: ffffabef43cdb8c0 R14: ffff8d504e850000 R15: 0000000000000000
>  FS:  00007f2c5462b080(0000) GS:ffff8d575f600000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000000000 CR3: 0000000167fd0000 CR4: 00000000007506f0
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   ? __die+0x20/0x70
>   ? page_fault_oops+0x15a/0x460
>   ? sched_balance_find_src_group+0x58d/0xd10
>   ? exc_page_fault+0x6e/0x180
>   ? asm_exc_page_fault+0x22/0x30
>   ? bnxt_get_queue_stats_rx+0xf/0x70 [bnxt_en cdd546fd48563c280cfd30e9647efa420db07bf1]
>   netdev_nl_stats_by_netdev+0x2b1/0x4e0
>   ? xas_load+0x9/0xb0
>   ? xas_find+0x183/0x1d0
>   ? xa_find+0x8b/0xe0
>   netdev_nl_qstats_get_dumpit+0xbf/0x1e0
>   genl_dumpit+0x31/0x90
>   netlink_dump+0x1a8/0x360
>
> Fixes: af7b3b4adda5 ("eth: bnxt: support per-queue statistics")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
>
> v3:
>  - Patch added.
>
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 9afb2c5072b1..bee12d9b57ab 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -15384,6 +15384,9 @@ static void bnxt_get_queue_stats_rx(struct net_device *dev, int i,
>         struct bnxt_cp_ring_info *cpr;
>         u64 *sw;
>
> +       if (!bp->bnapi)
> +               return;
> +
>         cpr = &bp->bnapi[i]->cp_ring;
>         sw = cpr->stats.sw_stats;
>
> @@ -15407,6 +15410,9 @@ static void bnxt_get_queue_stats_tx(struct net_device *dev, int i,
>         struct bnxt_napi *bnapi;
>         u64 *sw;
>
> +       if (!bp->tx_ring)
> +               return;
> +
>         bnapi = bp->tx_ring[bp->tx_ring_map[i]].bnapi;
>         sw = bnapi->cp_ring.stats.sw_stats;
>
> --
> 2.34.1
>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4199 bytes --]