[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrIb6zgbCxvc42HO@gmail.com>
Date: Tue, 6 Aug 2024 05:49:47 -0700
From: Breno Leitao <leitao@...ian.org>
To: Michael Chan <michael.chan@...adcom.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, pavan.chebbi@...adcom.com,
andrew.gospodarek@...adcom.com,
Kalesh AP <kalesh-anakkur.purayil@...adcom.com>,
Somnath Kotur <somnath.kotur@...adcom.com>
Subject: Re: [PATCH net] bnxt_en : Fix memory out-of-bounds in
bnxt_fill_hw_rss_tbl()
On Mon, Aug 05, 2024 at 10:37:42PM -0700, Michael Chan wrote:
> A recent commit has modified the code in __bnxt_reserve_rings() to
> set the default RSS indirection table to default only when the number
> of RX rings is changing. While this works for newer firmware that
> requires RX ring reservations, it causes the regression on older
> firmware not requiring RX ring resrvations (BNXT_NEW_RM() returns
> false).
>
> With older firmware, RX ring reservations are not required and so
> hw_resc->resv_rx_rings is not always set to the proper value. The
> comparison:
>
> if (old_rx_rings != bp->hw_resc.resv_rx_rings)
>
> in __bnxt_reserve_rings() may be false even when the RX rings are
> changing. This will cause __bnxt_reserve_rings() to skip setting
> the default RSS indirection table to default to match the current
> number of RX rings. This may later cause bnxt_fill_hw_rss_tbl() to
> use an out-of-range index.
>
> We already have bnxt_check_rss_tbl_no_rmgr() to handle exactly this
> scenario. We just need to move it up in bnxt_need_reserve_rings()
> to be called unconditionally when using older firmware. Without the
> fix, if the TX rings are changing, we'll skip the
> bnxt_check_rss_tbl_no_rmgr() call and __bnxt_reserve_rings() may also
> skip the bnxt_set_dflt_rss_indir_tbl() call for the reason explained
> in the last paragraph. Without setting the default RSS indirection
> table to default, it causes the regression:
>
> 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
>
> Reported-by: Breno Leitao <leitao@...ian.org>
> Closes: https://lore.kernel.org/netdev/ZrC6jpghA3PWVWSB@gmail.com/
> Fixes: 98ba1d931f61 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()")
> Reviewed-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@...adcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
Tested-by: Breno Leitao <leitao@...ian.org>
Powered by blists - more mailing lists