[PATCH net 2/3] net: hsr: clone before updating path and LAN IDs in tagged frames

[Xin Xie <xiexinet@gmail.com>]

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")
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 67aaf5a862..974b55f248 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