Re: [PATCH net-next v4 3/4] net: hsr: allow PRP RedBox (interlink) creation

[Xin Xie <xiexinet@gmail.com>]

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