* [PATCH net v2 1/3] net: hsr: privatize interlink-bound skbs before address mutation
2026-08-02 20:25 [PATCH net v2 0/3] net: hsr: fix shared-skb mutations in the forwarding path Xin Xie
@ 2026-08-02 20:25 ` Xin Xie
2026-08-02 20:25 ` [PATCH net v2 2/3] net: hsr: clone before updating path and LAN IDs in tagged frames Xin Xie
2026-08-02 20:25 ` [PATCH net v2 3/3] selftests: net: hsr: add shared-mutation regression test Xin Xie
2 siblings, 0 replies; 4+ messages in thread
From: Xin Xie @ 2026-08-02 20:25 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: edumazet, horms, shuah, lukma, m-karicheri2, fmaurer, luka.gejak,
bigeasy, ali, netdev, linux-kselftest, Xin Xie
On an HSR RedBox, frames forwarded to the interlink go through
hsr_get_untagged_frame() and arrive in hsr_xmit() as
skb_clone(frame->skb_std). Two reachable alias cases make the writes
there unsafe:
(a) A tagged multicast or broadcast frame received on a slave is
also delivered to the master through another clone of the same
buffer: hsr_deliver_master() writes the node MAC and hsr_xmit()
writes hsr->macaddress_redbox into the same six source bytes, so
the local stack receives the RedBox MAC instead of the
originating node's - deterministic without backpressure.
(b) A master-originated frame forwarded to the interlink aliases the
original TX skb, which packet taps or the TX path may still hold.
The master-origin substitutions (hsr_addr_subst_dest() and the
outgoing-slave source write) and the RedBox rewrite would all
modify that shared data.
Privatize the interlink-bound skb with skb_cow() before any address
mutation whenever it can alias a live consumer: for all
master-originated frames, and for ring frames the master also
consumes (is_local_dest && !is_local_exclusive - the exact inverse
of the port loop's master skip rules, so the condition does not
depend on port order). Every interlink skb reaching hsr_xmit() is a
clone from get_untagged_frame(), so the selected cases always pay
one private copy. On the hardware tag-insertion path, the selected
master-originated and locally consumed ring frames are now isolated
before the interlink address write: the interlink write can no
longer change an already queued clone of the same frame. Ring frames
the master does not consume (is_local_dest == false, e.g. unicast
ring-to-SAN) keep the zero-copy path; aliases that already exist
there - for example between the interlink clone and a peer slave's
hardware-tag-insertion clone - are pre-existing and unchanged by
this patch. On allocation failure the frame is dropped with the same
accounting as a tagged-frame creation failure.
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net v2 2/3] net: hsr: clone before updating path and LAN IDs in tagged frames
2026-08-02 20:25 [PATCH net v2 0/3] net: hsr: fix shared-skb mutations in the forwarding path Xin Xie
2026-08-02 20:25 ` [PATCH net v2 1/3] net: hsr: privatize interlink-bound skbs before address mutation Xin Xie
@ 2026-08-02 20:25 ` Xin Xie
2026-08-02 20:25 ` [PATCH net v2 3/3] selftests: net: hsr: add shared-mutation regression test Xin Xie
2 siblings, 0 replies; 4+ messages in thread
From: Xin Xie @ 2026-08-02 20:25 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: edumazet, horms, shuah, lukma, m-karicheri2, fmaurer, luka.gejak,
bigeasy, ali, netdev, linux-kselftest, Xin Xie
hsr_create_tagged_frame() and prp_create_tagged_frame() update the
path/LAN ID in the received skb data buffer and then skb_clone() it
for the egress port. When the same frame is forwarded to both slave
ports, the second port ID update lands in the same shared buffer the
first port clone still points at; if that clone is still queued
(qdisc backlog, NETEM, BQL), it is transmitted with the second port
ID - a silent on-wire corruption.
One always-available trigger is the master transmit path: a
pre-tagged frame transmitted on the master (for example injected
locally via AF_PACKET with a valid RCT) passes prp_fill_frame_info()
with frame->skb_prp set, the master-origin gate lets it through to
both slaves, and both slaves run prp_set_lan_id() + skb_clone() on
the same buffer. Tagged frames arriving on an HSR RedBox interlink
take the analogous path through hsr_create_tagged_frame(). Normal
traffic tests do not expose the race: the window between the first
dev_queue_xmit() and the second ID write is only a few instructions
and opens only under egress backpressure. With a 200 ms netem delay
on one slave, all 200 injected frames left that slave carrying the
other slave LAN ID in testing.
Reorder both helpers: clone first, privatize the clone with
skb_cow(), reacquire the header/trailer pointer, then update the ID.
skb_cow() is used rather than skb_cow_head() because privacy is
needed for the whole linear area, not only the header: the HSR tag
sits in the head, but the PRP RCT sits at the linear tail.
skb_get_PRP_rct() computes the trailer from skb_tail_pointer(), so
the returned pointer is always inside the linear area that
pskb_expand_head() copies; frames with a nonlinear tail are
mis-parsed by the existing helper regardless and are out of scope
here. (Both wrappers currently reach pskb_expand_head(); they differ
in the cloned-data predicate, and correctness here needs full data
privacy, not only header privacy.) This adds one linear-head copy
per tagged egress; untagged master traffic keeps the existing
__pskb_copy() path and pays nothing from this patch.
Fixes: 451d8123f897 ("net: prp: add packet handling support")
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 | 35 ++++++++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 67aaf5a8622b..974b55f24882 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -336,12 +336,24 @@ 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;
+
+ /* The original skb data may be shared with another egress
+ * clone. Make the clone data private before updating the
+ * path id so the update cannot corrupt the other copy.
+ */
+ skb = skb_clone(frame->skb_hsr, GFP_ATOMIC);
+ if (!skb)
+ return NULL;
+ if (skb_cow(skb, 0)) {
+ kfree_skb(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);
}
@@ -377,15 +389,28 @@ 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;
+ /* Same sharing hazard as above: privatize the clone data
+ * before updating the LAN id.
+ */
+ skb = skb_clone(frame->skb_prp, GFP_ATOMIC);
+ if (!skb)
+ return NULL;
+ if (skb_cow(skb, 0)) {
+ kfree_skb(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);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net v2 3/3] selftests: net: hsr: add shared-mutation regression test
2026-08-02 20:25 [PATCH net v2 0/3] net: hsr: fix shared-skb mutations in the forwarding path Xin Xie
2026-08-02 20:25 ` [PATCH net v2 1/3] net: hsr: privatize interlink-bound skbs before address mutation Xin Xie
2026-08-02 20:25 ` [PATCH net v2 2/3] net: hsr: clone before updating path and LAN IDs in tagged frames Xin Xie
@ 2026-08-02 20:25 ` Xin Xie
2 siblings, 0 replies; 4+ messages in thread
From: Xin Xie @ 2026-08-02 20:25 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: edumazet, horms, shuah, lukma, m-karicheri2, fmaurer, luka.gejak,
bigeasy, ali, netdev, linux-kselftest, Xin Xie
Cover both shared-skb mutation classes with one test. The PRP LAN-ID
case injects 200 pre-tagged frames on the master with one slave
egress delayed by netem and selects exactly the injected flow: on an
affected kernel all 200 frames leave slave A with slave B LAN ID
(FAIL); with the helpers privatizing clone data before the ID update
both sides stay isolated (PASS). The HSR RedBox source-MAC case
injects a tagged multicast data frame on a slave of an HSR RedBox:
on an affected kernel the local stack receives the RedBox MAC
instead of the originating node MAC (FAIL); with the interlink-bound
skb privatized before the rewrite, the master keeps the node MAC and
the interlink keeps the RedBox MAC (PASS).
Capture filters select destination, post-strip EtherType, and the
exact payload while leaving the asserted source MAC unfiltered.
Capability probes (tc, python3, sch_netem, HSR/PRP support) exit
ksft_skip and that status is propagated; real failures exit 1.
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
tools/testing/selftests/net/hsr/Makefile | 1 +
.../selftests/net/hsr/hsr_shared_mutation.sh | 223 ++++++++++++++++++
2 files changed, 224 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..05e7e803d286
--- /dev/null
+++ b/tools/testing/selftests/net/hsr/hsr_shared_mutation.sh
@@ -0,0 +1,223 @@
+#!/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)
+for _ in range(200):
+ tx.send(frame); time.sleep(0.001)
+
+a, b = [], []
+end = time.time() + dur
+while time.time() < end:
+ r, _, _ = select.select([sA, sB], [], [], 0.3)
+ for s in r:
+ pkt = s.recv(65535)
+ if (pkt[:6] != DST or pkt[6:12] != SRC or pkt[12:14] != b"\x08\x00"
+ or pkt[14:14 + len(PAY)] != PAY):
+ continue
+ lid = lanid(pkt)
+ if lid is not None:
+ (a if s is sA else b).append(lid)
+
+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):
+ tag = (((1 << 12) | len(PAY)).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=$?
+[ "$ret" -eq "$ksft_skip" ] && exit "$ksft_skip"
+[ "$ret" -eq 0 ] || rc=1
+
+run_f1
+ret=$?
+[ "$ret" -eq "$ksft_skip" ] && exit "$ksft_skip"
+[ "$ret" -eq 0 ] || rc=1
+
+if [ $rc -eq 0 ]; then
+ echo "hsr_shared_mutation: per-egress mutation isolation (F1+F2) [ OK ]"
+else
+ echo "hsr_shared_mutation: per-egress mutation isolation [ FAIL ]" \
+ "rc=$rc" 1>&2
+fi
+exit $rc
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread