This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at:
https://sashiko.dev/#/patchset/[email protected]?part=1
---
--- Patch [1]: [PATCH net-next v4 1/4] net: hsr: add PRP interlink (RedBox)
datapath and duplicate discard ---
commit 56647cd99caab3cbc966d1e4d258db2aadbec967
Author: Xin Xie <[email protected]>
net: hsr: add PRP interlink (RedBox) datapath and duplicate discard
This commit adds PRP interlink forwarding rules to prp_drop_frame() and gives
RedBox nodes a second duplicate-discard slot. It also publishes the RedBox
state before the first hsr_add_port() to prevent sequence state sizing issues.
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0774981a65c16..7734a521a96c2 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -440,12 +440,34 @@ static int hsr_xmit(struct sk_buff *skb, struct
> hsr_port *port,
> return dev_queue_xmit(skb);
> }
[Severity: High]
This isn't a bug introduced by this patch, but does modifying the MAC header
directly in hsr_xmit() corrupt shared packet data?
When an HSR frame is received from a slave port, it is forwarded to both the
master and interlink ports. Both ports receive a cloned skb that shares the
same underlying data buffer.
If we look at hsr_xmit(), the source MAC is modified directly:
net/hsr/hsr_forward.c:hsr_xmit() {
...
if (port->type == HSR_PT_INTERLINK)
ether_addr_copy(eth_hdr(skb)->h_source,
port->hsr->macaddress_redbox);
...
}
At the same time, hsr_deliver_master() modifies the source MAC on its clone
before delivering to the local stack via netif_rx().
Since both clones share the same buffer, does this overwrite the exact same
memory and corrupt the packet data for whichever interface gets queued first?
Should the skb be unshared before making these direct modifications?
Similarly, does hsr_create_tagged_frame() have the same issue when it
modifies the HSR tag path_id directly on frame->skb_hsr (the original
received packet)?
Since this path is triggered by incoming frames destined for the Redbox from
the network, can this reliable data corruption be reached by unprivileged
remote input?
