[PATCH net v3 0/3] net: hsr: fix shared-skb mutations in the forwarding path

[PATCH net v3 0/3] net: hsr: fix shared-skb mutations in the forwarding path

[Xin Xie]

Two existing HSR/PRP forwarding paths modify data shared by multiple
skbs. One changes RedBox interlink source addresses, and the other
updates per-egress HSR path or PRP LAN IDs. A later consumer can then
observe another consumer's value.

Patch 1 makes interlink address substitution private. Patch 2 uses one
clone/COW helper for tagged HSR/PRP frames and both hardware
tag-insertion branches. Patch 3 adds regressions for both corruptions.

The series is independent of the posted GRO/GSO v5 series:
https://lore.kernel.org/all/20260807140751.1351-1-xiexinet@gmail.com/
Both orders apply to current net. On current net-next, patch 3 needs
git am -3 because hsr_prp_redbox.sh is already in the same Makefile
block; the merged list remains sorted and validates.

Validation:
- the targeted test fails both cases on the base and passes on the
  candidate
- the affected HSR/PRP regression tests pass
- base-versus-candidate W=1 allmodconfig and allyesconfig add no
  diagnostics
- no TAG_INS hardware was available; those branches have source
  review only

Changes since v2, including official Sashiko dispositions:
- use a shared clone/COW helper and cover both TAG_INS branches
- correct the helper comment and interlink eligibility wording
- fix HSR LSDU encoding, capture draining, and result aggregation
- defer the existing dev->stats races per Paolo
- leave VLAN/non-linear receive parsing and receive-side tap aliases
  unchanged

Previous postings:
v2: https://lore.kernel.org/all/20260802202504.2962-1-xiexinet@gmail.com/
v1: https://lore.kernel.org/all/20260728143604.26-1-xiexinet@gmail.com/

Xin Xie (3):
  net: hsr: privatize interlink-bound skbs before address mutation
  net: hsr: return private clones from the tagged-frame helpers
  selftests: net: hsr: add shared-mutation regression test

 net/hsr/hsr_forward.c                         |  65 ++++-
 tools/testing/selftests/net/hsr/Makefile      |   1 +
 .../selftests/net/hsr/hsr_shared_mutation.sh  | 242 ++++++++++++++++++
 3 files changed, 299 insertions(+), 9 deletions(-)
 create mode 100755 tools/testing/selftests/net/hsr/hsr_shared_mutation.sh


base-commit: 594d905195024b228c962627ae5ae7c17bd582a4
-- 
2.43.0

[PATCH net v3 1/3] net: hsr: privatize interlink-bound skbs before address mutation

[Xin Xie]

An skb sent to a RedBox interlink can share data with master delivery
or the original TX skb. hsr_deliver_master() and hsr_xmit() then write
different source addresses, so one consumer can observe the other's
address.

Use skb_cow() before interlink address substitution when those paths
can share the data. Drop that egress on COW failure. Other interlink
traffic keeps its zero-copy behavior.

Fixes: 5055cccfc2d1 ("net: hsr: Provide RedBox support (HSR-SAN)")
Reviewed-by: Ali Ahmet Memis <ali@iusegentoo.com>
Tested-by: Ali Ahmet Memis <ali@iusegentoo.com>
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 net/hsr/hsr_forward.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 0774981a65c1..67aaf5a8622b 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -420,6 +420,22 @@ static void hsr_deliver_master(struct sk_buff *skb, struct net_device *dev,
 static int hsr_xmit(struct sk_buff *skb, struct hsr_port *port,
 		    struct hsr_frame_info *frame)
 {
+	/* An interlink-bound skb from get_untagged_frame() can still alias
+	 * another live consumer: for master-originated frames the clone
+	 * shares the original TX skb (which taps or the TX path may still
+	 * hold); for ring frames the master also consumes them when they
+	 * are destined to the local node without being exclusive to it.
+	 * Privatize before any address mutation.
+	 */
+	if (port->type == HSR_PT_INTERLINK &&
+	    (frame->port_rcv->type == HSR_PT_MASTER ||
+	     (frame->is_local_dest && !frame->is_local_exclusive)) &&
+	    skb_cow(skb, 0)) {
+		frame->port_rcv->dev->stats.rx_dropped++;
+		kfree_skb(skb);
+		return NET_XMIT_DROP;
+	}
+
 	if (frame->port_rcv->type == HSR_PT_MASTER) {
 		hsr_addr_subst_dest(frame->node_src, skb, port);
 
-- 
2.43.0

[PATCH net v3 2/3] net: hsr: return private clones from the tagged-frame helpers

[Xin Xie]

The tagged-frame helpers can return skbs whose data is still shared.
Later path, LAN ID, or source-address updates may then modify another
queued clone or the original TX skb.

Add hsr_clone_private() and use it for HSR, PRP, and both hardware
tag-insertion branches. Reacquire tag and trailer pointers after COW;
the PRP RCT is in the copied linear area.

This adds one linear-data copy per affected egress. Untagged software
forwarding is unchanged.

Fixes: 451d8123f897 ("net: prp: add packet handling support")
Fixes: dcf0cd1cc58b ("net: hsr: add offloading support")
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 net/hsr/hsr_forward.c | 49 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 67aaf5a8622b..efcbf3cf26f9 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -325,8 +325,29 @@ static struct sk_buff *hsr_fill_tag(struct sk_buff *skb,
 	return skb;
 }
 
-/* If the original frame was an HSR tagged frame, just clone it to be sent
- * unchanged. Otherwise, create a private frame especially tagged for 'port'.
+/* Clone an skb and make the clone's data private, so that per-egress
+ * writes cannot corrupt the original skb or other clones of it.
+ * Returns NULL on allocation failure.
+ */
+static struct sk_buff *hsr_clone_private(struct sk_buff *skb)
+{
+	struct sk_buff *clone;
+
+	clone = skb_clone(skb, GFP_ATOMIC);
+	if (!clone)
+		return NULL;
+	if (skb_cow(clone, 0)) {
+		kfree_skb(clone);
+		return NULL;
+	}
+
+	return clone;
+}
+
+/* If the original frame was an HSR tagged frame, return a private clone
+ * of it with the path id updated for 'port'. Otherwise, return a private
+ * clone for hardware tag insertion, or create a private frame especially
+ * tagged for 'port'.
  */
 struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 					struct hsr_port *port)
@@ -336,14 +357,18 @@ struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 	int movelen;
 
 	if (frame->skb_hsr) {
-		struct hsr_ethhdr *hsr_ethhdr =
-			(struct hsr_ethhdr *)skb_mac_header(frame->skb_hsr);
+		struct hsr_ethhdr *hsr_ethhdr;
+
+		skb = hsr_clone_private(frame->skb_hsr);
+		if (!skb)
+			return NULL;
 
 		/* set the lane id properly */
+		hsr_ethhdr = (struct hsr_ethhdr *)skb_mac_header(skb);
 		hsr_set_path_id(frame, hsr_ethhdr, port);
-		return skb_clone(frame->skb_hsr, GFP_ATOMIC);
+		return skb;
 	} else if (port->dev->features & NETIF_F_HW_HSR_TAG_INS) {
-		return skb_clone(frame->skb_std, GFP_ATOMIC);
+		return hsr_clone_private(frame->skb_std);
 	}
 
 	/* Create the new skb with enough headroom to fit the HSR tag */
@@ -377,17 +402,23 @@ struct sk_buff *prp_create_tagged_frame(struct hsr_frame_info *frame,
 	struct sk_buff *skb;
 
 	if (frame->skb_prp) {
-		struct prp_rct *trailer = skb_get_PRP_rct(frame->skb_prp);
+		struct prp_rct *trailer;
 
+		skb = hsr_clone_private(frame->skb_prp);
+		if (!skb)
+			return NULL;
+
+		trailer = skb_get_PRP_rct(skb);
 		if (trailer) {
 			prp_set_lan_id(trailer, port);
 		} else {
 			WARN_ONCE(!trailer, "errored PRP skb");
+			kfree_skb(skb);
 			return NULL;
 		}
-		return skb_clone(frame->skb_prp, GFP_ATOMIC);
+		return skb;
 	} else if (port->dev->features & NETIF_F_HW_HSR_TAG_INS) {
-		return skb_clone(frame->skb_std, GFP_ATOMIC);
+		return hsr_clone_private(frame->skb_std);
 	}
 
 	skb = skb_copy_expand(frame->skb_std, skb_headroom(frame->skb_std),
-- 
2.43.0

[PATCH net v3 3/3] selftests: net: hsr: add shared-mutation regression test

[Xin Xie]

Add regression coverage for both shared-data corruptions. Delay one
PRP egress and verify that queued clones retain their LAN IDs. For HSR
RedBox, deliver a tagged multicast to the master and interlink and
verify each source address.

Drain captures while injecting and merge both subtest results so
packet-socket pressure or a skip cannot hide a failure.

Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 tools/testing/selftests/net/hsr/Makefile      |   1 +
 .../selftests/net/hsr/hsr_shared_mutation.sh  | 242 ++++++++++++++++++
 2 files changed, 243 insertions(+)
 create mode 100755 tools/testing/selftests/net/hsr/hsr_shared_mutation.sh

diff --git a/tools/testing/selftests/net/hsr/Makefile b/tools/testing/selftests/net/hsr/Makefile
index 31fb9326cf53..87fe34951b8b 100644
--- a/tools/testing/selftests/net/hsr/Makefile
+++ b/tools/testing/selftests/net/hsr/Makefile
@@ -5,6 +5,7 @@ top_srcdir = ../../../../..
 TEST_PROGS := \
 	hsr_ping.sh \
 	hsr_redbox.sh \
+	hsr_shared_mutation.sh \
 	link_faults.sh \
 	prp_ping.sh \
 # end of TEST_PROGS
diff --git a/tools/testing/selftests/net/hsr/hsr_shared_mutation.sh b/tools/testing/selftests/net/hsr/hsr_shared_mutation.sh
new file mode 100755
index 000000000000..0b8b8791190d
--- /dev/null
+++ b/tools/testing/selftests/net/hsr/hsr_shared_mutation.sh
@@ -0,0 +1,242 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Verify that per-egress mutations of shared skb data are private:
+#
+# F2 (path/LAN ID): on an affected kernel the second slave's LAN-ID write
+# lands in the first slave's still-queued clone; with a netem delay on
+# slave A, injected frames leave A carrying B's LAN ID.
+#
+# F1 (RedBox source MAC): on an affected kernel an HSR-tagged multicast
+# frame received on a RedBox slave is cloned for master and interlink,
+# and the interlink's RedBox-MAC rewrite lands in the master clone's
+# buffer, so the local stack receives the RedBox MAC instead of the
+# originating node's MAC.
+
+ipv6=false
+
+source ./hsr_common.sh
+
+DUR=5
+
+require()
+{
+	command -v "$1" >/dev/null 2>&1 && return 0
+	echo "SKIP: $1 not available"
+	exit $ksft_skip
+}
+
+require ip
+require tc
+require python3
+
+trap cleanup_all_ns EXIT
+
+# ------------------------------------------------------- F2: LAN-ID isolation
+# PRP DANP (proto 1), AF_PACKET pre-tagged injection, netem on slave A.
+run_f2()
+{
+	setup_ns ns 2>/dev/null || return $ksft_skip
+	nsx() { ip netns exec "$ns" "$@"; }
+
+	# Probe sch_netem inside the disposable namespace only.
+	if ! nsx tc qdisc add dev lo root netem delay 1ms 2>/dev/null; then
+		echo "SKIP: sch_netem not available"
+		return $ksft_skip
+	fi
+	nsx tc qdisc del dev lo root 2>/dev/null
+
+	# Capability probes end here; setup or runtime failure below is FAIL.
+	nsx ip link add vA type veth peer name vAp ||
+		{ echo "FAIL: veth A"; return 1; }
+	nsx ip link add vB type veth peer name vBp ||
+		{ echo "FAIL: veth B"; return 1; }
+	for i in vA vB vAp vBp; do
+		nsx ip link set "$i" up || { echo "FAIL: $i up"; return 1; }
+	done
+	if ! nsx ip link add name prp0 type hsr slave1 vA slave2 vB \
+		supervision 45 proto 1 2>/dev/null; then
+		echo "SKIP: HSR/PRP not supported by this kernel"
+		return $ksft_skip
+	fi
+	nsx ip link set prp0 up || { echo "FAIL: prp0 up"; return 1; }
+	nsx tc qdisc add dev vA root netem delay 200ms ||
+		{ echo "FAIL: netem"; return 1; }
+
+	nsx python3 /dev/stdin "$DUR" <<'PYF2'
+import socket, struct, select, sys, time
+
+dur = int(sys.argv[1])
+def lanid(pkt):
+    if len(pkt) < 20 or pkt[-2:] != b"\x88\xfb":
+        return None
+    return (pkt[-4] >> 4) & 0xF
+
+SRC = bytes.fromhex(open("/sys/class/net/prp0/address").read().replace(":", ""))
+DST = bytes.fromhex("02aabbccdd01")
+PAY = bytes(range(46))
+rct0 = struct.pack(">H", 0) + struct.pack(">H", 52 & 0x0FFF) + b"\x88\xfb"
+frame = DST + SRC + b"\x08\x00" + PAY + rct0
+
+tx = socket.socket(socket.AF_PACKET, socket.SOCK_RAW); tx.bind(("prp0", 0))
+sA = socket.socket(socket.AF_PACKET, socket.SOCK_RAW,
+        socket.ntohs(0x0003))
+sA.bind(("vAp", 0))
+sB = socket.socket(socket.AF_PACKET, socket.SOCK_RAW,
+        socket.ntohs(0x0003))
+sB.bind(("vBp", 0))
+sA.setblocking(False); sB.setblocking(False)
+
+a, b = [], []
+
+def match(pkt):
+    return (pkt[:6] == DST and pkt[6:12] == SRC
+            and pkt[12:14] == b"\x08\x00" and pkt[14:14 + len(PAY)] == PAY)
+
+def drain(timeout):
+    wait = timeout
+    while True:
+        r, _, _ = select.select([sA, sB], [], [], wait)
+        if not r:
+            return
+        for s in r:
+            pkt = s.recv(65535)
+            if not match(pkt):
+                continue
+            lid = lanid(pkt)
+            if lid is not None:
+                (a if s is sA else b).append(lid)
+        wait = 0
+
+# Drain while sending so the burst cannot overflow the capture sockets'
+# receive buffers; the netem-delayed A-side frames arrive afterwards and
+# are collected below.
+for _ in range(200):
+    tx.send(frame)
+    drain(0)
+    time.sleep(0.001)
+
+end = time.time() + dur
+while time.time() < end:
+    drain(0.3)
+
+print("A-side count=%d lan ids=%s" % (len(a), sorted(set(a))))
+print("B-side count=%d lan ids=%s" % (len(b), sorted(set(b))))
+if len(a) < 150 or len(b) < 150:
+    print("FAIL: too few injected frames captured (A=%d B=%d, sent 200)"
+          % (len(a), len(b)))
+    sys.exit(1)
+bad_a = [x for x in a if (x & 1) != 0]
+bad_b = [x for x in b if (x & 1) != 1]
+if bad_a or bad_b:
+    print("FAIL: shared-mutation corruption - A: %d/%d wrong-lan,"
+          " B: %d/%d wrong-lan"
+          % (len(bad_a), len(a), len(bad_b), len(b)))
+    sys.exit(1)
+print("PASS: per-egress LAN IDs isolated (A all bit0=0, B all bit0=1)")
+sys.exit(0)
+PYF2
+}
+
+# --------------------------------------------- F1: RedBox source-MAC privacy
+# HSR RedBox (proto 0), tagged multicast from a slave: master must keep
+# the node MAC, interlink must carry the RedBox MAC.
+run_f1()
+{
+	setup_ns ns 2>/dev/null || return $ksft_skip
+	nsx() { ip netns exec "$ns" "$@"; }
+
+	nsx ip link add vA type veth peer name vAp ||
+		{ echo "FAIL: veth A"; return 1; }
+	nsx ip link add vB type veth peer name vBp ||
+		{ echo "FAIL: veth B"; return 1; }
+	nsx ip link add vI type veth peer name vIp ||
+		{ echo "FAIL: veth I"; return 1; }
+	for i in vA vB vI vAp vBp vIp; do
+		nsx ip link set "$i" up || { echo "FAIL: $i up"; return 1; }
+	done
+	if ! nsx ip link add name hsr0 type hsr slave1 vA slave2 vB \
+		interlink vI supervision 45 proto 0 2>/dev/null; then
+		echo "SKIP: HSR RedBox not supported by this kernel"
+		return $ksft_skip
+	fi
+	nsx ip link set hsr0 up || { echo "FAIL: hsr0 up"; return 1; }
+
+	nsx python3 /dev/stdin <<'PYF1'
+import socket, select, sys, time
+
+NODE  = bytes.fromhex("021122334455")
+MCAST = bytes.fromhex("01005e000001")
+RB    = bytes.fromhex(open("/sys/class/net/vI/address").read().replace(":", ""))
+PAY   = bytes(range(46))
+
+def frame(seq):
+    # LSDU size = payload + HSR tag (HSR_HLEN), as hsr_fill_tag() computes it
+    tag = (((1 << 12) | (len(PAY) + 6)).to_bytes(2, "big")
+           + seq.to_bytes(2, "big") + b"\x08\x00")
+    return MCAST + NODE + b"\x89\x2f" + tag + PAY
+
+tx = socket.socket(socket.AF_PACKET, socket.SOCK_RAW); tx.bind(("vAp", 0))
+sm = socket.socket(socket.AF_PACKET, socket.SOCK_RAW,
+        socket.ntohs(0x0003))
+sm.bind(("hsr0", 0))
+si = socket.socket(socket.AF_PACKET, socket.SOCK_RAW,
+        socket.ntohs(0x0003))
+si.bind(("vIp", 0))
+sm.setblocking(False); si.setblocking(False)
+
+for i in range(3):
+    tx.send(frame(i + 1)); time.sleep(0.05)
+
+m_src = i_src = None
+end = time.time() + 4
+while time.time() < end and (m_src is None or i_src is None):
+    r, _, _ = select.select([sm, si], [], [], 0.3)
+    for s in r:
+        pkt = s.recv(65535)
+        # exact flow: dst, post-strip EtherType, exact payload, min length;
+        # h_source is the asserted value and must NOT be filtered on
+        if (len(pkt) < 60 or pkt[:6] != MCAST or pkt[12:14] != b"\x08\x00"
+                or pkt[14:14 + len(PAY)] != PAY):
+            continue
+        if s is sm and m_src is None:
+            m_src = pkt[6:12]
+        elif s is si and i_src is None:
+            i_src = pkt[6:12]
+
+print("master h_source    =", m_src.hex() if m_src else None)
+print("node MAC           =", NODE.hex())
+print("interlink h_source =", i_src.hex() if i_src else None)
+print("redbox MAC         =", RB.hex())
+if i_src != RB:
+    print("FAIL: interlink did not carry the RedBox MAC")
+    sys.exit(1)
+if m_src != NODE:
+    print("FAIL: master received %s instead of the node MAC "
+          "(shared-mutation corruption)"
+          % (m_src.hex() if m_src else "nothing"))
+    sys.exit(1)
+print("PASS: master kept node MAC, interlink kept RedBox MAC")
+sys.exit(0)
+PYF1
+}
+
+rc=0
+
+run_f2
+ret=$?
+rc=$(ksft_status_merge "$rc" "$ret")
+
+run_f1
+ret=$?
+rc=$(ksft_status_merge "$rc" "$ret")
+
+if [ "$rc" -eq 0 ]; then
+	echo "hsr_shared_mutation: per-egress mutation isolation (F1+F2) [ OK ]"
+elif [ "$rc" -eq "$ksft_skip" ]; then
+	echo "hsr_shared_mutation: subtests skipped (capabilities missing)"
+else
+	echo "hsr_shared_mutation: per-egress mutation isolation [ FAIL ]" \
+		"rc=$rc" 1>&2
+fi
+exit "$rc"
-- 
2.43.0