Re: [PATCH net 1/2] net: hsr: Use the seqnr lock for frames received via interlink port.

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 2350 bytes --]

Hi Sebastian,

> syzbot reported that the seqnr_lock is not acquire for frames received
> over the interlink port. In the interlink case a new seqnr is
> generated and assigned to the frame.

Yes, correct.

The seq number for frames incomming from HSR ring are extracted from
the HSR header.

For frames going from interlink (SAN) network to RedBox, the start seq
number is assigned when node is created (and the creation node code is
reused).

> Frames, which are received over the slave port have already a sequence
> number assigned so the lock is not required.
> 
> Acquire the hsr_priv::seqnr_lock during in the invocation of
> hsr_forward_skb() if a packet has been received from the interlink
> port.
> 
> Reported-by: syzbot+3d602af7549af539274e@syzkaller.appspotmail.com
> Closes:
> https://groups.google.com/g/syzkaller-bugs/c/KppVvGviGg4/m/EItSdCZdBAAJ
> Fixes: 5055cccfc2d1c ("net: hsr: Provide RedBox support (HSR-SAN)")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> ---
>  net/hsr/hsr_slave.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index af6cf64a00e08..464f683e016db 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -67,7 +67,16 @@ static rx_handler_result_t hsr_handle_frame(struct
> sk_buff **pskb) skb_set_network_header(skb, ETH_HLEN + HSR_HLEN);
>  	skb_reset_mac_len(skb);
>  
> -	hsr_forward_skb(skb, port);
> +	/* Only the frames received over the interlink port will
> assign a
> +	 * sequence number and require synchronisation vs other
> sender.
> +	 */
> +	if (port->type == HSR_PT_INTERLINK) {
> +		spin_lock_bh(&hsr->seqnr_lock);

I'm just wondering if this could have impact on offloaded HSR operation.

I will try to run hsr_redbox.sh test on this patch (with QEMU) and
share results.

> +		hsr_forward_skb(skb, port);
> +		spin_unlock_bh(&hsr->seqnr_lock);
> +	} else {
> +		hsr_forward_skb(skb, port);
> +	}
>  
>  finish_consume:
>  	return RX_HANDLER_CONSUMED;




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]