[PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Jakub Kicinski]

bnxt doesn't check if a ring is used by RSS contexts when reducing
ring count. Core performs a similar check for the drivers for
the main context, but core doesn't know about additional contexts,
so it can't validate them. bnxt_fill_hw_rss_tbl_p5() uses ring
id to index bp->rx_ring[], which without the check may end up
being out of bounds.

  BUG: KASAN: slab-out-of-bounds in __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
  Read of size 2 at addr ffff8881c5809618 by task ethtool/31525
  Call Trace:
  __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
   bnxt_hwrm_vnic_rss_cfg_p5+0xf7/0x460
   __bnxt_setup_vnic_p5+0x12e/0x270
   __bnxt_open_nic+0x2262/0x2f30
   bnxt_open_nic+0x5d/0xf0
   ethnl_set_channels+0x5d4/0xb30
   ethnl_default_set_doit+0x2f1/0x620

Core does track the additional contexts in net-next, so we can
move this validation out of the driver as a follow up there.

Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: michael.chan@broadcom.com
CC: pavan.chebbi@broadcom.com
CC: kalesh-anakkur.purayil@broadcom.com
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 15 +++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 220d05e2f6fa..80fce0aaad66 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6282,6 +6282,21 @@ static u16 bnxt_get_max_rss_ring(struct bnxt *bp)
 	return max_ring;
 }
 
+u16 bnxt_get_max_rss_ctx_ring(struct bnxt *bp)
+{
+	u16 i, tbl_size, max_ring = 0;
+	struct bnxt_rss_ctx *rss_ctx;
+
+	tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
+
+	list_for_each_entry(rss_ctx, &bp->rss_ctx_list, list) {
+		for (i = 0; i < tbl_size; i++)
+			max_ring = max(max_ring, rss_ctx->rss_indir_tbl[i]);
+	}
+
+	return max_ring;
+}
+
 int bnxt_get_nr_rss_ctxs(struct bnxt *bp, int rx_rings)
 {
 	if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index e46bd11e52b0..3c8826875ceb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2814,6 +2814,7 @@ int bnxt_hwrm_vnic_set_tpa(struct bnxt *bp, struct bnxt_vnic_info *vnic,
 void bnxt_fill_ipv6_mask(__be32 mask[4]);
 int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
 void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
+u16 bnxt_get_max_rss_ctx_ring(struct bnxt *bp);
 int bnxt_get_nr_rss_ctxs(struct bnxt *bp, int rx_rings);
 int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic,
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index bf157f6cc042..4d53ec7adc61 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -961,6 +961,12 @@ static int bnxt_set_channels(struct net_device *dev,
 		return rc;
 	}
 
+	if (req_rx_rings < bp->rx_nr_rings &&
+	    req_rx_rings <= bnxt_get_max_rss_ctx_ring(bp)) {
+		netdev_warn(dev, "Can't deactivate rings used by RSS contexts\n");
+		return -EINVAL;
+	}
+
 	if (bnxt_get_nr_rss_ctxs(bp, req_rx_rings) !=
 	    bnxt_get_nr_rss_ctxs(bp, bp->rx_nr_rings) &&
 	    netif_is_rxfh_configured(dev)) {
-- 
2.45.2

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Pavan Chebbi]

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

On Fri, Jul 5, 2024 at 7:30 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> bnxt doesn't check if a ring is used by RSS contexts when reducing
> ring count. Core performs a similar check for the drivers for
> the main context, but core doesn't know about additional contexts,
> so it can't validate them. bnxt_fill_hw_rss_tbl_p5() uses ring
> id to index bp->rx_ring[], which without the check may end up
> being out of bounds.
>
>   BUG: KASAN: slab-out-of-bounds in __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
>   Read of size 2 at addr ffff8881c5809618 by task ethtool/31525
>   Call Trace:
>   __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
>    bnxt_hwrm_vnic_rss_cfg_p5+0xf7/0x460
>    __bnxt_setup_vnic_p5+0x12e/0x270
>    __bnxt_open_nic+0x2262/0x2f30
>    bnxt_open_nic+0x5d/0xf0
>    ethnl_set_channels+0x5d4/0xb30
>    ethnl_default_set_doit+0x2f1/0x620
>
> Core does track the additional contexts in net-next, so we can
> move this validation out of the driver as a follow up there.
>
> Fixes: b3d0083caf9a ("bnxt_en: Support RSS contexts in ethtool .{get|set}_rxfh()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: michael.chan@broadcom.com
> CC: pavan.chebbi@broadcom.com
> CC: kalesh-anakkur.purayil@broadcom.com
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 15 +++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  1 +
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  6 ++++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 220d05e2f6fa..80fce0aaad66 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6282,6 +6282,21 @@ static u16 bnxt_get_max_rss_ring(struct bnxt *bp)
>         return max_ring;
>  }
>
> +u16 bnxt_get_max_rss_ctx_ring(struct bnxt *bp)
> +{
> +       u16 i, tbl_size, max_ring = 0;
> +       struct bnxt_rss_ctx *rss_ctx;
> +
> +       tbl_size = bnxt_get_rxfh_indir_size(bp->dev);
> +
> +       list_for_each_entry(rss_ctx, &bp->rss_ctx_list, list) {
> +               for (i = 0; i < tbl_size; i++)
> +                       max_ring = max(max_ring, rss_ctx->rss_indir_tbl[i]);
> +       }
> +
> +       return max_ring;
> +}
> +
>  int bnxt_get_nr_rss_ctxs(struct bnxt *bp, int rx_rings)
>  {
>         if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS) {
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index e46bd11e52b0..3c8826875ceb 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2814,6 +2814,7 @@ int bnxt_hwrm_vnic_set_tpa(struct bnxt *bp, struct bnxt_vnic_info *vnic,
>  void bnxt_fill_ipv6_mask(__be32 mask[4]);
>  int bnxt_alloc_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
>  void bnxt_set_dflt_rss_indir_tbl(struct bnxt *bp, struct bnxt_rss_ctx *rss_ctx);
> +u16 bnxt_get_max_rss_ctx_ring(struct bnxt *bp);
>  int bnxt_get_nr_rss_ctxs(struct bnxt *bp, int rx_rings);
>  int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic);
>  int bnxt_hwrm_vnic_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic,
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index bf157f6cc042..4d53ec7adc61 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -961,6 +961,12 @@ static int bnxt_set_channels(struct net_device *dev,
>                 return rc;
>         }
>
> +       if (req_rx_rings < bp->rx_nr_rings &&
> +           req_rx_rings <= bnxt_get_max_rss_ctx_ring(bp)) {
> +               netdev_warn(dev, "Can't deactivate rings used by RSS contexts\n");
> +               return -EINVAL;
> +       }
> +
>         if (bnxt_get_nr_rss_ctxs(bp, req_rx_rings) !=
>             bnxt_get_nr_rss_ctxs(bp, bp->rx_nr_rings) &&
>             netif_is_rxfh_configured(dev)) {
> --
> 2.45.2
>
Thanks Jakub for the patch. This is much better than my earlier
thought of prioritizing ring change by destroying all the RSS ctxs.
Patch LGTM.
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

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

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu,  4 Jul 2024 19:00:05 -0700 you wrote:
> bnxt doesn't check if a ring is used by RSS contexts when reducing
> ring count. Core performs a similar check for the drivers for
> the main context, but core doesn't know about additional contexts,
> so it can't validate them. bnxt_fill_hw_rss_tbl_p5() uses ring
> id to index bp->rx_ring[], which without the check may end up
> being out of bounds.
> 
> [...]

Here is the summary with links:
  - [net] bnxt: fix crashes when reducing ring count with active RSS contexts
    https://git.kernel.org/netdev/net/c/0d1b7d6c9274

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Breno Leitao]

Hello,

On Thu, Jul 04, 2024 at 07:00:05PM -0700, Jakub Kicinski wrote:
> bnxt doesn't check if a ring is used by RSS contexts when reducing
> ring count. Core performs a similar check for the drivers for
> the main context, but core doesn't know about additional contexts,
> so it can't validate them. bnxt_fill_hw_rss_tbl_p5() uses ring
> id to index bp->rx_ring, which without the check may end up
> being out of bounds.
> 
>   BUG: KASAN: slab-out-of-bounds in __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
>   Read of size 2 at addr ffff8881c5809618 by task ethtool/31525
>   Call Trace:
>   __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
>    bnxt_hwrm_vnic_rss_cfg_p5+0xf7/0x460
>    __bnxt_setup_vnic_p5+0x12e/0x270
>    __bnxt_open_nic+0x2262/0x2f30
>    bnxt_open_nic+0x5d/0xf0
>    ethnl_set_channels+0x5d4/0xb30
>    ethnl_default_set_doit+0x2f1/0x620

I have this patch applied to my tree, and I am still finding a very
similar KASAN report in the last net-next/main tree - commit
3608d6aca5e793958462e6e01a8cdb6c6e8088d0 ("Merge branch 'dsa-en7581'
into main")

Skimmer over the code, In bnxt_fill_hw_rss_tbl(), bp->rss_indir_tbl[i]
returns 8, but, vnic->fw_grp_id size is 8, thus, it tries to access over
the last element (7).

Somehow bp->rss_indir_tbl[i] goes beynd rx_nr_rings.

--breno


	 ==================================================================
	 BUG: KASAN: slab-out-of-bounds in __bnxt_hwrm_vnic_set_rss (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6307 drivers/net/ethernet/broadcom/bnxt/bnxt.c:6347)
	 Read of size 2 at addr ffff88812c518f90 by task (udev-worker)/794

	 Call Trace:
	  <TASK>
	 dump_stack_lvl (lib/dump_stack.c:122)
	 print_report (mm/kasan/report.c:378 mm/kasan/report.c:488)
	 ? __virt_addr_valid (./arch/x86/include/asm/preempt.h:103 ./include/linux/rcupdate.h:953 ./include/linux/mmzone.h:2034 arch/x86/mm/physaddr.c:65)
	 ? __bnxt_hwrm_vnic_set_rss (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6307 drivers/net/ethernet/broadcom/bnxt/bnxt.c:6347)
	 kasan_report (mm/kasan/report.c:603)
	 ? __bnxt_hwrm_vnic_set_rss (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6307 drivers/net/ethernet/broadcom/bnxt/bnxt.c:6347)
	 __bnxt_hwrm_vnic_set_rss (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6307 drivers/net/ethernet/broadcom/bnxt/bnxt.c:6347)
	 ? _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
	 bnxt_hwrm_vnic_set_rss.part.0 (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6379)
	 ? __bnxt_hwrm_vnic_set_rss (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6364)
	 ? __bnxt_setup_vnic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:6624)
	 __bnxt_setup_vnic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:10073)
	 bnxt_init_nic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:10144 drivers/net/ethernet/broadcom/bnxt/bnxt.c:10336 drivers/net/ethernet/broadcom/bnxt/bnxt.c:10432)
	 ? bnxt_alloc_and_setup_vnic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:10425)
	 ? __irq_apply_affinity_hint (kernel/irq/manage.c:471 kernel/irq/manage.c:516)
	 ? irq_set_affinity_locked (kernel/irq/manage.c:507)
	 ? alloc_cpumask_var_node (lib/cpumask.c:62)
	 __bnxt_open_nic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:12103)
	 ? __netdev_update_features (net/core/dev.c:10116)
	 ? bnxt_init_one (drivers/net/ethernet/broadcom/bnxt/bnxt.c:12064)
	 ? __bnxt_close_nic.constprop.0 (drivers/net/ethernet/broadcom/bnxt/bnxt.c:10918 drivers/net/ethernet/broadcom/bnxt/bnxt.c:12323)
	 ? bnxt_set_channels (./arch/x86/include/asm/bitops.h:206 ./arch/x86/include/asm/bitops.h:238 ./include/asm-generic/bitops/instrumented-non-atomic.h:142 ./include/linux/netdevice.h:3588 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:1003)
	 bnxt_open_nic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:12179)
	 ethtool_set_channels (net/ethtool/ioctl.c:2117)
	 ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4995)
	 ? ethtool_set_settings (net/ethtool/ioctl.c:2065)
	 ? security_capable (security/security.c:1036 (discriminator 13))
	 __dev_ethtool (net/ethtool/ioctl.c:3275)
	 ? unwind_next_frame (arch/x86/kernel/unwind_orc.c:673)
	 ? arch_stack_walk (arch/x86/kernel/stacktrace.c:24)
	 ? ethtool_get_module_info_call (net/ethtool/ioctl.c:3044)
	 ? __lock_acquire (./arch/x86/include/asm/bitops.h:227 ./arch/x86/include/asm/bitops.h:239 ./include/asm-generic/bitops/instrumented-non-atomic.h:142 kernel/locking/lockdep.c:227 kernel/locking/lockdep.c:3780 kernel/locking/lockdep.c:3836 kernel/locking/lockdep.c:5142)
	 ? lockdep_hardirqs_on_prepare (kernel/locking/lockdep.c:4995)
	 ? stack_trace_save (kernel/stacktrace.c:123)
	 ? lock_acquire (kernel/locking/lockdep.c:466 kernel/locking/lockdep.c:5761 kernel/locking/lockdep.c:5724)
	 ? lock_sync (kernel/locking/lockdep.c:5727)
	 ? __kasan_kmalloc (mm/kasan/common.c:391)
	 ? dev_ethtool (net/ethtool/ioctl.c:3351)
	 ? dev_ioctl (net/core/dev_ioctl.c:721)
	 ? sock_ioctl (net/socket.c:1344)
	 ? rcu_is_watching (./include/linux/context_tracking.h:122 kernel/rcu/tree.c:726)
	 ? trace_contention_end (./include/trace/events/lock.h:122 (discriminator 52))
	 ? __mutex_lock (./arch/x86/include/asm/preempt.h:103 kernel/locking/mutex.c:618 kernel/locking/mutex.c:752)
	 ? lock_downgrade (kernel/locking/lockdep.c:5767)
	 ? dev_ethtool (net/ethtool/ioctl.c:3365)
	 ? sock_do_ioctl (net/socket.c:1237)
	 ? mutex_lock_io_nested (kernel/locking/mutex.c:751)
	 ? _raw_spin_unlock_irqrestore (./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
	 ? _raw_spin_unlock_irqrestore (./arch/x86/include/asm/preempt.h:103 ./include/linux/spinlock_api_smp.h:152 kernel/locking/spinlock.c:194)
	 ? rcu_is_watching (./include/linux/context_tracking.h:122 kernel/rcu/tree.c:726)
	 ? trace_kmalloc (./include/trace/events/kmem.h:54 (discriminator 52))
	 ? __kmalloc_cache_noprof (./include/linux/kasan.h:211 mm/slub.c:4189)
	 dev_ethtool (net/ethtool/ioctl.c:3365)
	 ? __dev_ethtool (net/ethtool/ioctl.c:3342)
	 dev_ioctl (net/core/dev_ioctl.c:721)
	 sock_do_ioctl (net/socket.c:1237)
	 ? put_user_ifreq (net/socket.c:1214)
	 ? find_held_lock (kernel/locking/lockdep.c:5249)
	 sock_ioctl (net/socket.c:1344)
	 ? br_ioctl_call (net/socket.c:1250)
	 ? seccomp_notify_ioctl (kernel/seccomp.c:1218)
	 ? ktime_get_coarse_real_ts64 (./include/linux/seqlock.h:74 kernel/time/timekeeping.c:2390)
	 ? lockdep_hardirqs_on (kernel/locking/lockdep.c:4420)
	 __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:907 fs/ioctl.c:893 fs/ioctl.c:893)
	 do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
	 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
	 RIP: 0033:0x7fab3150357b
	 Code: ff ff ff 85 c0 79 9b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 75 68 0f 00 f7 d8 64 89 01 48
	All code
	========
	   0:   ff                      (bad)
	   1:   ff                      (bad)
	   2:   ff 85 c0 79 9b 49       incl   0x499b79c0(%rbp)
	   8:   c7 c4 ff ff ff ff       mov    $0xffffffff,%esp
	   e:   5b                      pop    %rbx
	   f:   5d                      pop    %rbp
	  10:   4c 89 e0                mov    %r12,%rax
	  13:   41 5c                   pop    %r12
	  15:   c3                      ret
	  16:   66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
	  1d:   00 00
	  1f:   f3 0f 1e fa             endbr64
	  23:   b8 10 00 00 00          mov    $0x10,%eax
	  28:   0f 05                   syscall
	  2a:*  48 3d 01 f0 ff ff       cmp    $0xfffffffffffff001,%rax         <-- trapping instruction
	  30:   73 01                   jae    0x33
	  32:   c3                      ret
	  33:   48 8b 0d 75 68 0f 00    mov    0xf6875(%rip),%rcx        # 0xf68af
	  3a:   f7 d8                   neg    %eax
	  3c:   64 89 01                mov    %eax,%fs:(%rcx)
	  3f:   48                      rex.W

	Code starting with the faulting instruction
	===========================================
	   0:   48 3d 01 f0 ff ff       cmp    $0xfffffffffffff001,%rax
	   6:   73 01                   jae    0x9
	   8:   c3                      ret
	   9:   48 8b 0d 75 68 0f 00    mov    0xf6875(%rip),%rcx        # 0xf6885
	  10:   f7 d8                   neg    %eax
	  12:   64 89 01                mov    %eax,%fs:(%rcx)
	  15:   48                      rex.W
	 RSP: 002b:00007ffe53677a28 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
	 RAX: ffffffffffffffda RBX: 000055b5a1868cd8 RCX: 00007fab3150357b
	 RDX: 00007ffe53677a60 RSI: 0000000000008946 RDI: 000000000000001f
	 RBP: 00007ffe53677ab0 R08: 0000000000000000 R09: 0000000000000000
	 R10: 000055b5a18c9110 R11: 0000000000000246 R12: 000055b5a18c0ca0
	 R13: 000055b5a1866d18 R14: 00007ffe53677a60 R15: 000055b5a18becb0
	  </TASK>

	 Allocated by task 794:
	 kasan_save_stack (mm/kasan/common.c:48)
	 kasan_save_track (./arch/x86/include/asm/current.h:49 mm/kasan/common.c:60 mm/kasan/common.c:69)
	 __kasan_kmalloc (mm/kasan/common.c:391)
	 __kmalloc_noprof (mm/slub.c:4159 mm/slub.c:4170)
	 bnxt_alloc_mem (drivers/net/ethernet/broadcom/bnxt/bnxt.c:4696 drivers/net/ethernet/broadcom/bnxt/bnxt.c:5323)
	 __bnxt_open_nic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:12088)
	 bnxt_open_nic (drivers/net/ethernet/broadcom/bnxt/bnxt.c:12179)
	 ethtool_set_channels (net/ethtool/ioctl.c:2117)
	 __dev_ethtool (net/ethtool/ioctl.c:3275)
	 dev_ethtool (net/ethtool/ioctl.c:3365)
	 dev_ioctl (net/core/dev_ioctl.c:721)
	 sock_do_ioctl (net/socket.c:1237)
	 sock_ioctl (net/socket.c:1344)
	 __x64_sys_ioctl (fs/ioctl.c:52 fs/ioctl.c:907 fs/ioctl.c:893 fs/ioctl.c:893)
	 do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
	 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

	 The buggy address belongs to the object at ffff88812c518f80
	which belongs to the cache kmalloc-16 of size 16
	 The buggy address is located 0 bytes to the right of
	allocated 16-byte region [ffff88812c518f80, ffff88812c518f90)

	 The buggy address belongs to the physical page:
	 page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x12c518
	 anon flags: 0x5ffff0000000000(node=0|zone=2|lastcpupid=0x1ffff)
	 page_type: 0xfdffffff(slab)
	 raw: 05ffff0000000000 ffff88810004c640 0000000000000000 0000000000000001
	 raw: 0000000000000000 0000000080800080 00000001fdffffff 0000000000000000
	 page dumped because: kasan: bad access detected

	 Memory state around the buggy address:
	  ffff88812c518e80: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc
	  ffff88812c518f00: fa fb fc fc fa fb fc fc fa fb fc fc fa fb fc fc
	 >ffff88812c518f80: 00 00 fc fc 00 00 fc fc 00 01 fc fc fa fb fc fc
				  ^
	  ffff88812c519000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	  ffff88812c519080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
	 ==================================================================

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Breno Leitao]

On Mon, Aug 05, 2024 at 04:42:11AM -0700, Breno Leitao wrote:
> Hello,
> 
> On Thu, Jul 04, 2024 at 07:00:05PM -0700, Jakub Kicinski wrote:
> > bnxt doesn't check if a ring is used by RSS contexts when reducing
> > ring count. Core performs a similar check for the drivers for
> > the main context, but core doesn't know about additional contexts,
> > so it can't validate them. bnxt_fill_hw_rss_tbl_p5() uses ring
> > id to index bp->rx_ring, which without the check may end up
> > being out of bounds.
> > 
> >   BUG: KASAN: slab-out-of-bounds in __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
> >   Read of size 2 at addr ffff8881c5809618 by task ethtool/31525
> >   Call Trace:
> >   __bnxt_hwrm_vnic_set_rss+0xb79/0xe40
> >    bnxt_hwrm_vnic_rss_cfg_p5+0xf7/0x460
> >    __bnxt_setup_vnic_p5+0x12e/0x270
> >    __bnxt_open_nic+0x2262/0x2f30
> >    bnxt_open_nic+0x5d/0xf0
> >    ethnl_set_channels+0x5d4/0xb30
> >    ethnl_default_set_doit+0x2f1/0x620
> 
> I have this patch applied to my tree, and I am still finding a very
> similar KASAN report in the last net-next/main tree - commit
> 3608d6aca5e793958462e6e01a8cdb6c6e8088d0 ("Merge branch 'dsa-en7581'
> into main")
> 
> Skimmer over the code, In bnxt_fill_hw_rss_tbl(), bp->rss_indir_tbl[i]
> returns 8, but, vnic->fw_grp_id size is 8, thus, it tries to access over
> the last element (7).
> 
> Somehow bp->rss_indir_tbl[i] goes beynd rx_nr_rings.

I was able to debug this further, and the culprit is

98ba1d931f611e8f8f519c040 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()")

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Michael Chan]

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

On Mon, Aug 5, 2024 at 11:04 AM Breno Leitao <leitao@debian.org> wrote:
>
> On Mon, Aug 05, 2024 at 04:42:11AM -0700, Breno Leitao wrote:
> > Somehow bp->rss_indir_tbl[i] goes beynd rx_nr_rings.
>
> I was able to debug this further, and the culprit is
>
> 98ba1d931f611e8f8f519c040 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()")

Thanks, we'll look into it.  Do you have the steps to reproduce it?

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

Re: [PATCH net] bnxt: fix crashes when reducing ring count with active RSS contexts

[Michael Chan]

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

On Mon, Aug 5, 2024 at 11:23 AM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Mon, Aug 5, 2024 at 11:04 AM Breno Leitao <leitao@debian.org> wrote:
> >
> > On Mon, Aug 05, 2024 at 04:42:11AM -0700, Breno Leitao wrote:
> > > Somehow bp->rss_indir_tbl[i] goes beynd rx_nr_rings.
> >
> > I was able to debug this further, and the culprit is
> >
> > 98ba1d931f611e8f8f519c040 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()")
>
> Thanks, we'll look into it.  Do you have the steps to reproduce it?

Maybe the FW on your NIC is older and BNXT_NEW_RM() is false.  That
code path may not be correct.  Anyway, we'll look into it.  Thanks
again.

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