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

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: <ZrEUGypU4miANEQ9@gmail.com>
Date: Mon, 5 Aug 2024 11:04:11 -0700
From: Breno Leitao <leitao@...ian.org>
To: Jakub Kicinski <kuba@...nel.org>, michael.chan@...adcom.com,
	pavan.chebbi@...adcom.com
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
	pabeni@...hat.com, kalesh-anakkur.purayil@...adcom.com
Subject: Re: [PATCH net] bnxt: fix crashes when reducing ring count with
 active RSS contexts

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()")