[PATCH net] net: macb: take bp->lock around NCR read-modify-writes
From: Théo Lebrun <theo.lebrun@bootlin.com>
Date: 2026-09-18 19:59:47
Also in:
lkml, stable
Subsystem:
atmel macb ethernet driver, networking drivers, the rest · Maintainers:
Théo Lebrun, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
NCR is read-modify-written from many contexts:
- macb_mac_link_down() clears RE|TE,
- macb_mac_link_up() sets RE|TE|PTPUNI,
- macb_hresp_error_task() clears then re-sets RE|TE,
- macb_start_xmit() / macb_tx_restart() / macb_tx_error_task() set
TSTART (already under bp->lock),
- macb_interrupt() might toggle RE (also under bp->lock).
The first three risk concurrent RMW with anyone from the list as they
don't grab bp->lock.
Theoretical bugfix only, it has never reproduced on hardware. Also note
it does *not* take the lock hoping to protect against full races
inbetween BH scheduling concurrently. We only want to ensure consistent
NCR RMW operations.
Fixes: 7897b071ac3b ("net: macb: convert to phylink")
Cc: stable@vger.kernel.org
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/macb_main.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..b58d3f01f2cc 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c@@ -777,6 +777,7 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode, struct net_device *netdev = to_net_dev(config->dev); struct macb *bp = netdev_priv(netdev); struct macb_queue *queue; + unsigned long flags; unsigned int q; u32 ctrl;
@@ -786,8 +787,10 @@ static void macb_mac_link_down(struct phylink_config *config, unsigned int mode, bp->rx_intr_mask | MACB_TX_INT_FLAGS | MACB_BIT(HRESP)); /* Disable Rx and Tx */ + spin_lock_irqsave(&bp->lock, flags); ctrl = macb_readl(bp, NCR) & ~(MACB_BIT(RE) | MACB_BIT(TE)); macb_writel(bp, NCR, ctrl); + spin_unlock_irqrestore(&bp->lock, flags); netif_tx_stop_all_queues(netdev); }
@@ -940,11 +943,13 @@ static void macb_mac_link_up(struct phylink_config *config, } /* Enable Rx and Tx; Enable PTP unicast */ + spin_lock_irqsave(&bp->lock, flags); ctrl = macb_readl(bp, NCR); if (gem_has_ptp(bp)) ctrl |= MACB_BIT(PTPUNI); macb_writel(bp, NCR, ctrl | MACB_BIT(RE) | MACB_BIT(TE)); + spin_unlock_irqrestore(&bp->lock, flags); netif_tx_wake_all_queues(netdev); }
@@ -1995,6 +2000,7 @@ static void macb_hresp_error_task(struct work_struct *work) struct macb *bp = from_work(bp, work, hresp_err_bh_work); struct net_device *netdev = bp->netdev; struct macb_queue *queue; + unsigned long flags; unsigned int q; u32 ctrl;
@@ -2003,9 +2009,11 @@ static void macb_hresp_error_task(struct work_struct *work) MACB_TX_INT_FLAGS | MACB_BIT(HRESP)); } + spin_lock_irqsave(&bp->lock, flags); ctrl = macb_readl(bp, NCR); ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE)); macb_writel(bp, NCR, ctrl); + spin_unlock_irqrestore(&bp->lock, flags); netif_tx_stop_all_queues(netdev); netif_carrier_off(netdev);
@@ -2022,8 +2030,10 @@ static void macb_hresp_error_task(struct work_struct *work) MACB_TX_INT_FLAGS | MACB_BIT(HRESP)); - ctrl |= MACB_BIT(RE) | MACB_BIT(TE); - macb_writel(bp, NCR, ctrl); + spin_lock_irqsave(&bp->lock, flags); + ctrl = MACB_BIT(RE) | MACB_BIT(TE); + macb_writel(bp, NCR, macb_readl(bp, NCR) | ctrl); + spin_unlock_irqrestore(&bp->lock, flags); netif_carrier_on(netdev); netif_tx_start_all_queues(netdev);
--- base-commit: 994db8ab9d90c64dd641b7ead6efe2eaea7a50dc change-id: 20260918-macb-ncr-rmw-be5137e60f0b Best regards, -- Théo Lebrun [off-list ref]