Re: [PATCH 1/1] net: hsr: avoid learning nodes from ordinary PRP SAN traffic

Paolo Abeni
2026-04-07T08:40:35Z

On 4/2/26 4:18 PM, Ao Zhou wrote:
> From: Haoze Xie <royenheart@gmail.com>
>
> PRP slave ports accept ordinary SAN traffic and forward it to the
> master device without needing a persistent node entry. Creating one
> node per previously unseen SAN source lets arbitrary source MAC floods
> grow node_db until the prune timer catches up.
>
> Keep the receive path for ordinary PRP SAN traffic, but stop learning a
> new node when the frame is untagged and does not carry a PRP trailer.
> Continue to deliver the frame locally and only keep node state for
> actual HSR/PRP senders or nodes that have already been learned.

It's not clear to me what prevents the rouge sender from always
including a valid PRP trailer, thus still causing the issue addressed
here. Some rationale needs to be included, or possibly limit the amount
of newly created nodes to some configurable maximum.

> Fixes: 451d8123f897 ("net: prp: add packet handling support")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Yuqi Xu <xuyuqiabc@gmail.com>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
> ---
> net/hsr/hsr_forward.c | 14 ++++++++++----
> net/hsr/hsr_framereg.c | 16 +++++++++++++---
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index aefc9b6936ba..5fbfc42997d2 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -403,7 +403,8 @@ static void hsr_deliver_master(struct sk_buff *skb, struct net_device *dev,
> int res, recv_len;
>
> was_multicast_frame = (skb->pkt_type == PACKET_MULTICAST);
> - hsr_addr_subst_source(node_src, skb);
> + if (node_src)
> + hsr_addr_subst_source(node_src, skb);
> skb_pull(skb, ETH_HLEN);
> recv_len = skb->len;
> res = netif_rx(skb);
> @@ -699,8 +700,12 @@ static int fill_frame_info(struct hsr_frame_info *frame,
>
> frame->node_src = hsr_get_node(port, n_db, skb,
> frame->is_supervision, port->type);
> - if (!frame->node_src)
> - return -1; /* Unknown node and !is_supervision, or no mem */
> + if (IS_ERR(frame->node_src)) {
> + ret = PTR_ERR(frame->node_src);
> + if (ret != -ENOENT)
> + return ret;
> + frame->node_src = NULL;
> + }
>
> ethhdr = (struct ethhdr *)skb_mac_header(skb);
> frame->is_vlan = false;
> @@ -739,7 +744,8 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
> if (fill_frame_info(&frame, skb, port) < 0)
> goto out_drop;
>
> - hsr_register_frame_in(frame.node_src, port, frame.sequence_nr);
> + if (frame.node_src)
> + hsr_register_frame_in(frame.node_src, port, frame.sequence_nr);
> hsr_forward_do(&frame);

I *think* the hsr_forward_do() could unconditionally dereference
node_src in:

hsr->proto_ops->register_frame_out -> {prp,hsr}_register_frame_out ->
hsr_check_duplicate:

node = frame->node_src;
if (WARN_ON_ONCE(port_type >= node->seq_port_cnt))

/P