netdev - [PATCH net] net: tsnep: fix timestamping with a stacked DSA driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250512132430.344473-1-vladimir.oltean@nxp.com>
Date: Mon, 12 May 2025 16:24:30 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Gerhard Engleder <gerhard@...leder-embedded.com>,
	Andrew Lunn <andrew@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	Richard Cochran <richardcochran@...il.com>
Subject: [PATCH net] net: tsnep: fix timestamping with a stacked DSA driver

This driver seems susceptible to a form of the bug explained in commit
c26a2c2ddc01 ("gianfar: Fix TX timestamping with a stacked DSA driver")
and in Documentation/networking/timestamping.rst section "Other caveats
for MAC drivers", specifically it timestamps any skb which has
SKBTX_HW_TSTAMP, and does not consider adapter->hwtstamp_config.tx_type.

Evaluate the proper TX timestamping condition only once on the TX
path (in tsnep_netdev_xmit_frame()) and pass it down to
tsnep_xmit_frame_ring() and tsnep_tx_activate() through a bool variable.

Also evaluate it again in the TX confirmation path, in tsnep_tx_poll(),
since I don't know whether TSNEP_DESC_EXTENDED_WRITEBACK_FLAG is a
confounding condition and may be set for other reasons than hardware
timestamping too.

Fixes: 403f69bbdbad ("tsnep: Add TSN endpoint Ethernet MAC driver")
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 drivers/net/ethernet/engleder/tsnep_main.c | 25 ++++++++++++++--------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 625245b0845c..00eb570e026e 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -377,7 +377,7 @@ static void tsnep_tx_disable(struct tsnep_tx *tx, struct napi_struct *napi)
 }
 
 static void tsnep_tx_activate(struct tsnep_tx *tx, int index, int length,
-			      bool last)
+			      bool last, bool do_tstamp)
 {
 	struct tsnep_tx_entry *entry = &tx->entry[index];
 
@@ -386,8 +386,7 @@ static void tsnep_tx_activate(struct tsnep_tx *tx, int index, int length,
 	if (entry->skb) {
 		entry->properties = length & TSNEP_DESC_LENGTH_MASK;
 		entry->properties |= TSNEP_DESC_INTERRUPT_FLAG;
-		if ((entry->type & TSNEP_TX_TYPE_SKB) &&
-		    (skb_shinfo(entry->skb)->tx_flags & SKBTX_IN_PROGRESS))
+		if (do_tstamp)
 			entry->properties |= TSNEP_DESC_EXTENDED_WRITEBACK_FLAG;
 
 		/* toggle user flag to prevent false acknowledge
@@ -556,7 +555,8 @@ static int tsnep_tx_unmap(struct tsnep_tx *tx, int index, int count)
 }
 
 static netdev_tx_t tsnep_xmit_frame_ring(struct sk_buff *skb,
-					 struct tsnep_tx *tx)
+					 struct tsnep_tx *tx,
+					 bool do_tstamp)
 {
 	int count = 1;
 	struct tsnep_tx_entry *entry;
@@ -591,12 +591,12 @@ static netdev_tx_t tsnep_xmit_frame_ring(struct sk_buff *skb,
 	}
 	length = retval;
 
-	if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
+	if (do_tstamp)
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
 	for (i = 0; i < count; i++)
 		tsnep_tx_activate(tx, (tx->write + i) & TSNEP_RING_MASK, length,
-				  i == count - 1);
+				  i == count - 1, do_tstamp);
 	tx->write = (tx->write + count) & TSNEP_RING_MASK;
 
 	skb_tx_timestamp(skb);
@@ -704,7 +704,7 @@ static bool tsnep_xdp_xmit_frame_ring(struct xdp_frame *xdpf,
 
 	for (i = 0; i < count; i++)
 		tsnep_tx_activate(tx, (tx->write + i) & TSNEP_RING_MASK, length,
-				  i == count - 1);
+				  i == count - 1, false);
 	tx->write = (tx->write + count) & TSNEP_RING_MASK;
 
 	/* descriptor properties shall be valid before hardware is notified */
@@ -775,7 +775,7 @@ static void tsnep_xdp_xmit_frame_ring_zc(struct xdp_desc *xdpd,
 
 	length = tsnep_xdp_tx_map_zc(xdpd, tx);
 
-	tsnep_tx_activate(tx, tx->write, length, true);
+	tsnep_tx_activate(tx, tx->write, length, true, false);
 	tx->write = (tx->write + 1) & TSNEP_RING_MASK;
 }
 
@@ -845,6 +845,7 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
 		length = tsnep_tx_unmap(tx, tx->read, count);
 
 		if ((entry->type & TSNEP_TX_TYPE_SKB) &&
+		    (tx->adapter->hwtstamp_config.tx_type == HWTSTAMP_TX_ON) &&
 		    (skb_shinfo(entry->skb)->tx_flags & SKBTX_IN_PROGRESS) &&
 		    (__le32_to_cpu(entry->desc_wb->properties) &
 		     TSNEP_DESC_EXTENDED_WRITEBACK_FLAG)) {
@@ -2153,11 +2154,17 @@ static netdev_tx_t tsnep_netdev_xmit_frame(struct sk_buff *skb,
 {
 	struct tsnep_adapter *adapter = netdev_priv(netdev);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
+	bool do_tstamp = false;
 
 	if (queue_mapping >= adapter->num_tx_queues)
 		queue_mapping = 0;
 
-	return tsnep_xmit_frame_ring(skb, &adapter->tx[queue_mapping]);
+	if (adapter->hwtstamp_config.tx_type == HWTSTAMP_TX_ON &&
+	    skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
+		do_tstamp = true;
+
+	return tsnep_xmit_frame_ring(skb, &adapter->tx[queue_mapping],
+				     do_tstamp);
 }
 
 static int tsnep_netdev_ioctl(struct net_device *netdev, struct ifreq *ifr,
-- 
2.43.0