| Message ID | 20240705020005.681746-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] bnxt: fix crashes when reducing ring count with active RSS contexts | expand |
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>
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)) {
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(+)