On Fri, 24 Jul 2026 14:12:46 +0100 Simon Horman wrote: > [Severity: Medium] > ... unconditionally increment the port's tx_packets and tx_bytes > ... inflate the network statistics for the physical interlink > device?
Confirmed against the v4 base. hsr_forward_skb() receives the ingress port; when that is the interlink, the interlink device's tx_packets/tx_bytes are incremented for a frame that arrived from a SAN and is forwarded towards LAN A/B - nothing is transmitted on that device. A deterministic accounting error, pre-existing since v6.11; the series does not touch any statistics code. I will send an independent fix for net. > [Severity: Medium] > ... err_unregister ... omits a call to hsr_del_nodes(). ... Confirmed. The earlier hsr_add_port() calls install live RX handlers, and RTNL does not exclude softirq RX, so dynamic nodes can be learned before a later hsr_add_port() fails. err_unregister removes the ports and the self node but never calls hsr_del_nodes(), and unregister_netdevice() does not invoke hsr_dellink(). A real leak, though a narrow one: it needs a partial-initialization failure after traffic has already flowed. Pre-existing (identical in v6.11), unchanged by this series. I will send an independent fix for net. > [Severity: High] > ... directly modify the path or LAN ID in the data buffer of the > original shared SKB before cloning it. Does the second > modification for the second slave overwrite the first ... ? This needs two separate answers. The specific sequence the review describes - tagged input on the interlink forwarded to both slaves - does not occur in the PRP control flow: * A normal SAN frame received on the interlink is untagged: handle_std_frame() sets frame->skb_std, and each LAN output goes through the skb_copy_expand() branch, so LAN A and LAN B receive independent packet data. * An already PRP-tagged frame on the interlink sets frame->skb_prp, but is_from_san remains false, so hsr_forward_do() consults prp_register_frame_out() before any helper runs. For LAN A or LAN B as destination it returns 1 and the loop continues, so neither LAN reaches prp_create_tagged_frame(). (This series touches that gate only to open the interlink destination, which is served by get_untagged_frame(), never by the tagged-frame helper.) * A PRP-tagged frame received from one LAN is not forwarded to the other LAN either (the same gate), so it cannot produce two slave calls on the same frame->skb_prp. The underlying pattern is nevertheless reachable, on the master transmit path: a pre-tagged frame transmitted on the master device (for example injected locally via AF_PACKET with a valid RCT; this needs CAP_NET_RAW) passes prp_fill_frame_info() with frame->skb_prp set. With port_rcv == MASTER the gate returns 0, and both slaves then run prp_set_lan_id() + skb_clone() on the same buffer, so the second LAN-id write can land in the first clone's data while it is still queued. hsr_create_tagged_frame() has the analogous shape for HSR. That path is pre-existing (the helper has had this shape since at least v6.11) and is not introduced by v4, but the concern itself is valid. I will fix it independently for net, together with the other items. To summarise: all four findings describe pre-existing code that this series does not modify. The interlink-ingress sequence from the last finding does not occur for PRP, but the same modify-before-clone pattern is reachable on the master transmit path, so that concern joins the other three: four independent fixes for net, each with its own verified Fixes: commit - I do not assume they share an originating commit. Would you like v4 to wait for the fixes, or may it proceed while they are reviewed separately? -- Xin
