Skip to content

Commit c86b08d

Browse files
committed
eth: bnxt: always recalculate features after XDP clearing, fix null-deref
JIRA: https://issues.redhat.com/browse/RHEL-107291 CVE: CVE-2025-21682 commit f0aa6a3 Author: Jakub Kicinski <kuba@kernel.org> Date: Wed Jan 8 20:30:57 2025 -0800 eth: bnxt: always recalculate features after XDP clearing, fix null-deref Recalculate features when XDP is detached. Before: # ip li set dev eth0 xdp obj xdp_dummy.bpf.o sec xdp # ip li set dev eth0 xdp off # ethtool -k eth0 | grep gro rx-gro-hw: off [requested on] After: # ip li set dev eth0 xdp obj xdp_dummy.bpf.o sec xdp # ip li set dev eth0 xdp off # ethtool -k eth0 | grep gro rx-gro-hw: on The fact that HW-GRO doesn't get re-enabled automatically is just a minor annoyance. The real issue is that the features will randomly come back during another reconfiguration which just happens to invoke netdev_update_features(). The driver doesn't handle reconfiguring two things at a time very robustly. Starting with commit 98ba1d9 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") we only reconfigure the RSS hash table if the "effective" number of Rx rings has changed. If HW-GRO is enabled "effective" number of rings is 2x what user sees. So if we are in the bad state, with HW-GRO re-enablement "pending" after XDP off, and we lower the rings by / 2 - the HW-GRO rings doing 2x and the ethtool -L doing / 2 may cancel each other out, and the: if (old_rx_rings != bp->hw_resc.resv_rx_rings && condition in __bnxt_reserve_rings() will be false. The RSS map won't get updated, and we'll crash with: BUG: kernel NULL pointer dereference, address: 0000000000000168 RIP: 0010:__bnxt_hwrm_vnic_set_rss+0x13a/0x1a0 bnxt_hwrm_vnic_rss_cfg_p5+0x47/0x180 __bnxt_setup_vnic_p5+0x58/0x110 bnxt_init_nic+0xb72/0xf50 __bnxt_open_nic+0x40d/0xab0 bnxt_open_nic+0x2b/0x60 ethtool_set_channels+0x18c/0x1d0 As we try to access a freed ring. The issue is present since XDP support was added, really, but prior to commit 98ba1d9 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") it wasn't causing major issues. Fixes: 1054aee ("bnxt_en: Use NETIF_F_GRO_HW.") Fixes: 98ba1d9 ("bnxt_en: Fix RSS logic in __bnxt_reserve_rings()") Reviewed-by: Michael Chan <michael.chan@broadcom.com> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com> Link: https://patch.msgid.link/20250109043057.2888953-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
1 parent 99edb26 commit c86b08d

File tree

3 files changed

+21
-13
lines changed

3 files changed

+21
-13
lines changed

drivers/net/ethernet/broadcom/bnxt/bnxt.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4743,7 +4743,7 @@ void bnxt_set_ring_params(struct bnxt *bp)
47434743
/* Changing allocation mode of RX rings.
47444744
* TODO: Update when extending xdp_rxq_info to support allocation modes.
47454745
*/
4746-
int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
4746+
static void __bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
47474747
{
47484748
struct net_device *dev = bp->dev;
47494749

@@ -4764,15 +4764,30 @@ int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
47644764
bp->rx_skb_func = bnxt_rx_page_skb;
47654765
}
47664766
bp->rx_dir = DMA_BIDIRECTIONAL;
4767-
/* Disable LRO or GRO_HW */
4768-
netdev_update_features(dev);
47694767
} else {
47704768
dev->max_mtu = bp->max_mtu;
47714769
bp->flags &= ~BNXT_FLAG_RX_PAGE_MODE;
47724770
bp->rx_dir = DMA_FROM_DEVICE;
47734771
bp->rx_skb_func = bnxt_rx_skb;
47744772
}
4775-
return 0;
4773+
}
4774+
4775+
void bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
4776+
{
4777+
__bnxt_set_rx_skb_mode(bp, page_mode);
4778+
4779+
if (!page_mode) {
4780+
int rx, tx;
4781+
4782+
bnxt_get_max_rings(bp, &rx, &tx, true);
4783+
if (rx > 1) {
4784+
bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS;
4785+
bp->dev->hw_features |= NETIF_F_LRO;
4786+
}
4787+
}
4788+
4789+
/* Update LRO and GRO_HW availability */
4790+
netdev_update_features(bp->dev);
47764791
}
47774792

47784793
static void bnxt_free_vnic_attributes(struct bnxt *bp)
@@ -16450,7 +16465,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
1645016465
if (bp->max_fltr < BNXT_MAX_FLTR)
1645116466
bp->max_fltr = BNXT_MAX_FLTR;
1645216467
bnxt_init_l2_fltr_tbl(bp);
16453-
bnxt_set_rx_skb_mode(bp, false);
16468+
__bnxt_set_rx_skb_mode(bp, false);
1645416469
bnxt_set_tpa_flags(bp);
1645516470
bnxt_init_ring_params(bp);
1645616471
bnxt_set_ring_params(bp);

drivers/net/ethernet/broadcom/bnxt/bnxt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2876,7 +2876,7 @@ u32 bnxt_fw_health_readl(struct bnxt *bp, int reg_idx);
28762876
bool bnxt_bs_trace_avail(struct bnxt *bp, u16 type);
28772877
void bnxt_set_tpa_flags(struct bnxt *bp);
28782878
void bnxt_set_ring_params(struct bnxt *);
2879-
int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode);
2879+
void bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode);
28802880
void bnxt_insert_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr);
28812881
void bnxt_del_one_usr_fltr(struct bnxt *bp, struct bnxt_filter_base *fltr);
28822882
int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp, unsigned long *bmap,

drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -425,15 +425,8 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
425425
bnxt_set_rx_skb_mode(bp, true);
426426
xdp_features_set_redirect_target(dev, true);
427427
} else {
428-
int rx, tx;
429-
430428
xdp_features_clear_redirect_target(dev);
431429
bnxt_set_rx_skb_mode(bp, false);
432-
bnxt_get_max_rings(bp, &rx, &tx, true);
433-
if (rx > 1) {
434-
bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS;
435-
bp->dev->hw_features |= NETIF_F_LRO;
436-
}
437430
}
438431
bp->tx_nr_rings_xdp = tx_xdp;
439432
bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tc + tx_xdp;

0 commit comments

Comments
 (0)