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

[Paolo Abeni <pabeni@redhat.com>]

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