From 1b6fd1161f475e2e12672da2605f0de0dccf0809 Mon Sep 17 00:00:00 2001 From: "Ilya A. Evenbach" Date: Tue, 22 Jul 2025 13:29:47 -0700 Subject: [PATCH 1/2] [sparx5] Use SKB reference counting for PTP tx path There is a race condition in the TX path of PTP event packets. When the packet is transmitted, DMA is kicked off and then SKB is freed. However, in case of PTP packet, SKB is queued for timestamping, and is freed in the interrupt handler. If PTP IRQ arrives before the call to sparx5_consume_skb(), it will free the packet, and then sparx5_consume_skb() will cause Oops, when testing whether SKB is PTP or not. This change uses SKB reference counting to ensure SKB is freed regardless of order of events, and no crash occurs. --- .../microchip/sparx5/lan969x/lan969x_fdma.c | 3 +-- .../microchip/sparx5/lan969x/lan969x_fdma_pci.c | 7 +------ .../net/ethernet/microchip/sparx5/sparx5_fdma.c | 2 +- .../net/ethernet/microchip/sparx5/sparx5_main.h | 1 - .../net/ethernet/microchip/sparx5/sparx5_packet.c | 14 +------------- drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c | 8 ++++++++ 6 files changed, 12 insertions(+), 23 deletions(-) diff --git a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma.c b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma.c index 11cfb8871b8a..23172057d2c0 100644 --- a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma.c +++ b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma.c @@ -77,8 +77,7 @@ static void lan969x_fdma_tx_clear_buf(struct sparx5 *sparx5, int weight) db->len, DMA_TO_DEVICE); - if (!db->ptp) - napi_consume_skb(db->skb, weight); + napi_consume_skb(db->skb, weight); break; case SPX5_DB_DATA_TYPE_XDPF: /* XDP_REDIRECT or AF_XDP */ diff --git a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma_pci.c b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma_pci.c index d5229849cd0b..65a45eb23fa2 100644 --- a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma_pci.c +++ b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_fdma_pci.c @@ -299,12 +299,7 @@ int lan969x_fdma_pci_xmit(struct sparx5 *sparx5, u32 *ifh, struct sk_buff *skb) sparx5_fdma_reload(sparx5, fdma); } - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && - SPARX5_SKB_CB(skb)->rew_op == IFH_REW_OP_TWO_STEP_PTP) - ptp = true; - - if (!ptp) - dev_consume_skb_any(skb); + dev_consume_skb_any(skb); return NETDEV_TX_OK; } diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c index 0ada7be59c09..2cdba8613181 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c @@ -299,7 +299,7 @@ int sparx5_fdma_xmit(struct sparx5 *sparx5, u32 *ifh, struct sk_buff *skb) sparx5_fdma_reload(sparx5, fdma); - sparx5_consume_skb(skb); + dev_consume_skb_any(skb); return NETDEV_TX_OK; } diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h index 499cc00b25d6..b197945700c4 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h @@ -1081,7 +1081,6 @@ u32 sparx5_mirror_monitor_get(struct sparx5 *sparx5, u32 idx); /* sparx5_packet.c */ u32 sparx5_get_packet_pipeline_pt(enum sparx5_packet_pipeline_pt pt); -void sparx5_consume_skb(struct sk_buff *skb); bool sparx5_skb_offloaded(struct sparx5 *sparx5, u32 port, struct sk_buff *skb); /* sparx5_afi.c */ diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c index e39f16d9f22f..c5649a870e96 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c @@ -276,7 +276,7 @@ static int sparx5_inject(struct sparx5 *sparx5, HRTIMER_MODE_REL); } - sparx5_consume_skb(skb); + dev_consume_skb_any(skb); return NETDEV_TX_OK; } @@ -448,18 +448,6 @@ void sparx5_port_inj_timer_setup(struct sparx5_port *port) port->inj_timer.function = sparx5_injection_timeout; } -void sparx5_consume_skb(struct sk_buff *skb) -{ - bool ptp = false; - - if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && - SPARX5_SKB_CB(skb)->rew_op == IFH_REW_OP_TWO_STEP_PTP) - ptp = true; - - if (!ptp) - dev_consume_skb_any(skb); -} - bool sparx5_skb_offloaded(struct sparx5 *sparx5, u32 port, struct sk_buff *skb) { u32 val; diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c index a6fb3621e9eb..c39014a03254 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c @@ -568,6 +568,13 @@ int sparx5_ptp_txtstamp_request(struct sparx5_port *port, } skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; + /* Increase reference count, in order to prevent double-free, + in case PTP IRQ happens after DMA is kicked off, but before + dev_consume_skb_any is called. + For PTP packets, skb is freed extra time by the PTP IRQ handler, + so we need to increase the reference count here. + */ + skb_get(skb); skb_queue_tail(&port->tx_skbs, skb); SPARX5_SKB_CB(skb)->ts_id = port->ts_id; @@ -593,6 +600,7 @@ void sparx5_ptp_txtstamp_release(struct sparx5_port *port, port->ts_id--; sparx5->ptp_skbs--; skb_unlink(skb, &port->tx_skbs); + dev_kfree_skb_any(skb); spin_unlock_irqrestore(&sparx5->ptp_ts_id_lock, flags); } From ca3756d1db07dc688aca9c86a4d446199914be3b Mon Sep 17 00:00:00 2001 From: "Ilya A. Evenbach" Date: Tue, 26 Aug 2025 10:47:48 -0700 Subject: [PATCH 2/2] [sparx5] Pad the SKB to minimum size before calling request_tx_ts For PTP event packets, sparx5_ptp_txtstamp_request increases reference count, which makes skb_put_padto BUG() - reference count should be exactly one for that one. --- drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 5 ----- drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c | 5 ----- drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 5 +++++ drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c | 3 --- drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 6 ++++++ 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c index 91738f0e4bd3..1fb953b0eafc 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c @@ -717,11 +717,6 @@ int lan966x_fdma_xmit(struct sk_buff *skb, __be32 *ifh, struct net_device *dev) return NETDEV_TX_BUSY; } - if (skb_put_padto(skb, ETH_ZLEN)) { - dev->stats.tx_dropped++; - return NETDEV_TX_OK; - } - /* skb processing */ needed_headroom = max_t(int, IFH_LEN_BYTES - skb_headroom(skb), 0); needed_tailroom = max_t(int, ETH_FCS_LEN - skb_tailroom(skb), 0); diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c index 96af9fbf0523..175b216dd6f3 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c @@ -283,11 +283,6 @@ static int lan966x_fdma_pci_xmit(struct sk_buff *skb, __be32 *ifh, return NETDEV_TX_BUSY; } - if (skb_put_padto(skb, ETH_ZLEN)) { - dev->stats.tx_dropped++; - return NETDEV_TX_OK; - } - skb_tx_timestamp(skb); virt_addr = fdma_dataptr_virt_get_contiguous(fdma, next_to_use, 0); diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c index 6bf1ccdf71f6..66fcd35aa30f 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c @@ -387,6 +387,11 @@ netdev_tx_t lan966x_xmit(struct lan966x_port *port, ops = &lan966x->data->ops; + if (skb_put_padto(skb, ETH_ZLEN)) { + dev->stats.tx_dropped++; + return NETDEV_TX_OK; + } + spin_lock(&lan966x->tx_lock); if (port->lan966x->fdma) err = ops->fdma_xmit(skb, ifh, port->dev); diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c index 2cdba8613181..7cf8d535fe29 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_fdma.c @@ -280,9 +280,6 @@ int sparx5_fdma_xmit(struct sparx5 *sparx5, u32 *ifh, struct sk_buff *skb) fdma_dcb_advance(fdma); - if (skb_put_padto(skb, ETH_ZLEN)) - return NETDEV_TX_OK; - if (!fdma_db_is_done(fdma_db_get(fdma, fdma->dcb_index, 0))) return NETDEV_TX_BUSY; diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c index c5649a870e96..33f2e7137de3 100644 --- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c @@ -297,6 +297,9 @@ netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev) sparx5_set_port_ifh(sparx5, ifh, port->portno, SPX5_PACKET_PIPELINE_PT_ANA_DONE); + if (skb_put_padto(skb, ETH_ZLEN)) + return NETDEV_TX_OK; + if (sparx5->ptp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) { if (sparx5_ptp_txtstamp_request(port, skb) < 0) return NETDEV_TX_BUSY; @@ -355,6 +358,9 @@ netdev_tx_t sparx5_port_xmit(struct sparx5_port *port, struct sk_buff *skb, ops = &sparx5->data->ops; + if (skb_put_padto(skb, ETH_ZLEN)) + return NETDEV_TX_OK; + spin_lock(&sparx5->tx_lock); if (sparx5->fdma_irq > 0) ret = ops->fdma_xmit(sparx5, ifh, skb);