| Message ID | 20240904133725.1073963-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b3c9e65eb227269ed72a115ba22f4f51b4e62b4d |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: hsr: remove seqnr_lock | expand |
On Wed, Sep 04, 2024 at 01:37:25PM +0000, Eric Dumazet wrote: > syzbot found a new splat [1]. > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > This also avoid a race in hsr_fill_info(). > > Also remove interlink_sequence_nr which is unused. > > [1] > WARNING: CPU: 1 PID: 9723 at net/hsr/hsr_forward.c:602 handle_std_frame+0x247/0x2c0 net/hsr/hsr_forward.c:602 > Modules linked in: > CPU: 1 UID: 0 PID: 9723 Comm: syz.0.1657 Not tainted 6.11.0-rc6-syzkaller-00026-g88fac17500f4 #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 > RIP: 0010:handle_std_frame+0x247/0x2c0 net/hsr/hsr_forward.c:602 > Code: 49 8d bd b0 01 00 00 be ff ff ff ff e8 e2 58 25 00 31 ff 89 c5 89 c6 e8 47 53 a8 f6 85 ed 0f 85 5a ff ff ff e8 fa 50 a8 f6 90 <0f> 0b 90 e9 4c ff ff ff e8 cc e7 06 f7 e9 8f fe ff ff e8 52 e8 06 > RSP: 0018:ffffc90000598598 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: ffffc90000598670 RCX: ffffffff8ae2c919 > RDX: ffff888024e94880 RSI: ffffffff8ae2c926 RDI: 0000000000000005 > RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000003 > R13: ffff8880627a8cc0 R14: 0000000000000000 R15: ffff888012b03c3a > FS: 0000000000000000(0000) GS:ffff88802b700000(0063) knlGS:00000000f5696b40 > CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033 > CR2: 0000000020010000 CR3: 00000000768b4000 CR4: 0000000000350ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <IRQ> > hsr_fill_frame_info+0x2c8/0x360 net/hsr/hsr_forward.c:630 > fill_frame_info net/hsr/hsr_forward.c:700 [inline] > hsr_forward_skb+0x7df/0x25c0 net/hsr/hsr_forward.c:715 > hsr_handle_frame+0x603/0x850 net/hsr/hsr_slave.c:70 > __netif_receive_skb_core.constprop.0+0xa3d/0x4330 net/core/dev.c:5555 > __netif_receive_skb_list_core+0x357/0x950 net/core/dev.c:5737 > __netif_receive_skb_list net/core/dev.c:5804 [inline] > netif_receive_skb_list_internal+0x753/0xda0 net/core/dev.c:5896 > gro_normal_list include/net/gro.h:515 [inline] > gro_normal_list include/net/gro.h:511 [inline] > napi_complete_done+0x23f/0x9a0 net/core/dev.c:6247 > gro_cell_poll+0x162/0x210 net/core/gro_cells.c:66 > __napi_poll.constprop.0+0xb7/0x550 net/core/dev.c:6772 > napi_poll net/core/dev.c:6841 [inline] > net_rx_action+0xa92/0x1010 net/core/dev.c:6963 > handle_softirqs+0x216/0x8f0 kernel/softirq.c:554 > do_softirq kernel/softirq.c:455 [inline] > do_softirq+0xb2/0xf0 kernel/softirq.c:442 > </IRQ> > <TASK> > > Fixes: 06afd2c31d33 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") > Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Thanks Eric, I see that; 1) The stack trace above is for a call path where hsr_forward_skb(), and in turn handle_std_frame(), is called without hsr->seqnr_lock held, which explains the splat. 2) Access to hsr->sequence_nr in hsr_fill_info() is not synchronised with other accesses to it. 3) hsr->interlink_sequence_nr is set but otherwise unknown And that this patch addresses all of the above. Reviewed-by: Simon Horman <horms@kernel.org>
On 2024-09-04 13:37:25 [+0000], Eric Dumazet wrote: > syzbot found a new splat [1]. > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > This also avoid a race in hsr_fill_info(). You obtain to sequence nr without locking so two CPUs could submit skbs at the same time. Wouldn't this allow the race I described in commit 06afd2c31d338 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") to happen again? Then one skb would be dropped while sending because it has lower sequence nr but in fact it was not yet sent. Sebastian
On Thu, Sep 5, 2024 at 2:17 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2024-09-04 13:37:25 [+0000], Eric Dumazet wrote: > > syzbot found a new splat [1]. > > > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > > > This also avoid a race in hsr_fill_info(). > > You obtain to sequence nr without locking so two CPUs could submit skbs > at the same time. Wouldn't this allow the race I described in commit > 06afd2c31d338 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") > > to happen again? Then one skb would be dropped while sending because it > has lower sequence nr but in fact it was not yet sent. > A network protocol unable to cope with reorders can not live really. If this is an issue, this should be fixed at the receiving side.
On 2024-09-05 14:26:30 [+0200], Eric Dumazet wrote: > On Thu, Sep 5, 2024 at 2:17 PM Sebastian Andrzej Siewior > <bigeasy@linutronix.de> wrote: > > > > On 2024-09-04 13:37:25 [+0000], Eric Dumazet wrote: > > > syzbot found a new splat [1]. > > > > > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > > > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > > > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > > > > > This also avoid a race in hsr_fill_info(). > > > > You obtain to sequence nr without locking so two CPUs could submit skbs > > at the same time. Wouldn't this allow the race I described in commit > > 06afd2c31d338 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") > > > > to happen again? Then one skb would be dropped while sending because it > > has lower sequence nr but in fact it was not yet sent. > > > > A network protocol unable to cope with reorders can not live really. > > If this is an issue, this should be fixed at the receiving side. The standard/ network protocol just says there has to be seq nr to avoid processing a packet multiple times. The Linux implementation increments the counter and assumes everything lower than the current counter has already been seen. Therefore the sequence lock is held during the entire sending process. This is not a protocol but implementation issue ;) I am aware of a FPGA implementation of HSR which tracks the last 20 sequence numbers instead. This would help because it would allow reorders to happen. Looking at it, the code chain never held the lock while I was playing with it and I did not see this. So this might be just a consequence of using gro here. I don't remember disabling it so it must have been of by default or syzbot found a way to enable it (or has better hardware). Would it make sense to disable this for HSR interfaces? Sebastian
On Thu, Sep 5, 2024 at 3:18 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2024-09-05 14:26:30 [+0200], Eric Dumazet wrote: > > On Thu, Sep 5, 2024 at 2:17 PM Sebastian Andrzej Siewior > > <bigeasy@linutronix.de> wrote: > > > > > > On 2024-09-04 13:37:25 [+0000], Eric Dumazet wrote: > > > > syzbot found a new splat [1]. > > > > > > > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > > > > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > > > > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > > > > > > > This also avoid a race in hsr_fill_info(). > > > > > > You obtain to sequence nr without locking so two CPUs could submit skbs > > > at the same time. Wouldn't this allow the race I described in commit > > > 06afd2c31d338 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") > > > > > > to happen again? Then one skb would be dropped while sending because it > > > has lower sequence nr but in fact it was not yet sent. > > > > > > > A network protocol unable to cope with reorders can not live really. > > > > If this is an issue, this should be fixed at the receiving side. > > The standard/ network protocol just says there has to be seq nr to avoid > processing a packet multiple times. The Linux implementation increments > the counter and assumes everything lower than the current counter has > already been seen. Therefore the sequence lock is held during the entire > sending process. > This is not a protocol but implementation issue ;) These packets are sent on a physical network, reorders are inevitable. > I am aware of a FPGA implementation of HSR which tracks the last 20 > sequence numbers instead. This would help because it would allow > reorders to happen. > > Looking at it, the code chain never held the lock while I was playing > with it and I did not see this. So this might be just a consequence of > using gro here. I don't remember disabling it so it must have been of by > default or syzbot found a way to enable it (or has better hardware). > > Would it make sense to disable this for HSR interfaces? This has nothing to do with GRO. Look at this alternative patch, perhaps you will see the problem ? diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c index af6cf64a00e081c777db5f7786e8a27ea6f62e14..3971dbc0644ab8d32c04c262dbba7b1c950ebea9 100644 --- a/net/hsr/hsr_slave.c +++ b/net/hsr/hsr_slave.c @@ -67,7 +67,9 @@ 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); + spin_lock_bh(&hsr->seqnr_lock); hsr_forward_skb(skb, port); + spin_unlock_bh(&hsr->seqnr_lock); finish_consume: return RX_HANDLER_CONSUMED; I am surprised we even have a discussion considering HSR has Orphan status in MAINTAINERS... I do not know how to test HSR, I am not sure the alternative patch is correct. Removing the seqnr_lock seems the safest to me.
On 2024-09-05 15:26:27 [+0200], Eric Dumazet wrote: > diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c > index af6cf64a00e081c777db5f7786e8a27ea6f62e14..3971dbc0644ab8d32c04c262dbba7b1c950ebea9 > 100644 > --- a/net/hsr/hsr_slave.c > +++ b/net/hsr/hsr_slave.c > @@ -67,7 +67,9 @@ 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); > > + spin_lock_bh(&hsr->seqnr_lock); > hsr_forward_skb(skb, port); > + spin_unlock_bh(&hsr->seqnr_lock); > > finish_consume: > return RX_HANDLER_CONSUMED; > > > I am surprised we even have a discussion considering HSR has Orphan > status in MAINTAINERS... > > I do not know how to test HSR, I am not sure the alternative patch is correct. I did submit something to tests somewhere. I will try to test this and let you know. > Removing the seqnr_lock seems the safest to me. Consider this as ack if you don't hear back from me. Sebastian
On 2024-09-05 15:26:27 [+0200], Eric Dumazet wrote: > > This has nothing to do with GRO. > > Look at this alternative patch, perhaps you will see the problem ? > > diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c > index af6cf64a00e081c777db5f7786e8a27ea6f62e14..3971dbc0644ab8d32c04c262dbba7b1c950ebea9 > 100644 > --- a/net/hsr/hsr_slave.c > +++ b/net/hsr/hsr_slave.c > @@ -67,7 +67,9 @@ 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); > > + spin_lock_bh(&hsr->seqnr_lock); > hsr_forward_skb(skb, port); > + spin_unlock_bh(&hsr->seqnr_lock); > > finish_consume: > return RX_HANDLER_CONSUMED; > This does not trigger any warning while testing (warning as in recursion or so). The other invocations have the lock so it should work. This did not trigger earlier because hsr_handle_frame() is invoked from the Slave-Interfaces which don't assign a seq-nr. syzkaller might have managed to receive the packet from the master interface. Or it is the interlink which is new and was added in commit 5055cccfc2d1c ("net: hsr: Provide RedBox support (HSR-SAN)"). Did the bot leave a reproducer? I'm wondering if the packet is dropped later in process. I can't test interlink right now, my `ip' seems not recent enough. Assuming it is a interlink packet, I would suggest to acquire that lock as you suggested. I can send a patch if you wish. Added Lukasz Majewski on Cc who added interlink support, maybe he can say if this is a legal path. Sebastian
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Wed, 4 Sep 2024 13:37:25 +0000 you wrote: > syzbot found a new splat [1]. > > Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / > spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock > and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. > > This also avoid a race in hsr_fill_info(). > > [...] Here is the summary with links: - [net] net: hsr: remove seqnr_lock https://git.kernel.org/netdev/net/c/b3c9e65eb227 You are awesome, thank you!
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c index e4cc6b78dcfc40643ad386780573a53dd089bb54..ac56784c327c0293fef13e3ecfa9fc32ca1ab839 100644 --- a/net/hsr/hsr_device.c +++ b/net/hsr/hsr_device.c @@ -231,9 +231,7 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev) skb->dev = master->dev; skb_reset_mac_header(skb); skb_reset_mac_len(skb); - spin_lock_bh(&hsr->seqnr_lock); hsr_forward_skb(skb, master); - spin_unlock_bh(&hsr->seqnr_lock); } else { dev_core_stats_tx_dropped_inc(dev); dev_kfree_skb_any(skb); @@ -314,14 +312,10 @@ static void send_hsr_supervision_frame(struct hsr_port *port, set_hsr_stag_HSR_ver(hsr_stag, hsr->prot_version); /* From HSRv1 on we have separate supervision sequence numbers. */ - spin_lock_bh(&hsr->seqnr_lock); - if (hsr->prot_version > 0) { - hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr); - hsr->sup_sequence_nr++; - } else { - hsr_stag->sequence_nr = htons(hsr->sequence_nr); - hsr->sequence_nr++; - } + if (hsr->prot_version > 0) + hsr_stag->sequence_nr = htons(atomic_inc_return(&hsr->sup_sequence_nr)); + else + hsr_stag->sequence_nr = htons(atomic_inc_return(&hsr->sequence_nr)); hsr_stag->tlv.HSR_TLV_type = type; /* TODO: Why 12 in HSRv0? */ @@ -343,13 +337,11 @@ static void send_hsr_supervision_frame(struct hsr_port *port, ether_addr_copy(hsr_sp->macaddress_A, hsr->macaddress_redbox); } - if (skb_put_padto(skb, ETH_ZLEN)) { - spin_unlock_bh(&hsr->seqnr_lock); + if (skb_put_padto(skb, ETH_ZLEN)) return; - } hsr_forward_skb(skb, port); - spin_unlock_bh(&hsr->seqnr_lock); + return; } @@ -374,9 +366,7 @@ static void send_prp_supervision_frame(struct hsr_port *master, set_hsr_stag_HSR_ver(hsr_stag, (hsr->prot_version ? 1 : 0)); /* From HSRv1 on we have separate supervision sequence numbers. */ - spin_lock_bh(&hsr->seqnr_lock); - hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr); - hsr->sup_sequence_nr++; + hsr_stag->sequence_nr = htons(atomic_inc_return(&hsr->sup_sequence_nr)); hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD; hsr_stag->tlv.HSR_TLV_length = sizeof(struct hsr_sup_payload); @@ -384,13 +374,10 @@ static void send_prp_supervision_frame(struct hsr_port *master, hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload)); ether_addr_copy(hsr_sp->macaddress_A, master->dev->dev_addr); - if (skb_put_padto(skb, ETH_ZLEN)) { - spin_unlock_bh(&hsr->seqnr_lock); + if (skb_put_padto(skb, ETH_ZLEN)) return; - } hsr_forward_skb(skb, master); - spin_unlock_bh(&hsr->seqnr_lock); } /* Announce (supervision frame) timer function @@ -621,11 +608,9 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], if (res < 0) return res; - spin_lock_init(&hsr->seqnr_lock); /* Overflow soon to find bugs easier: */ - hsr->sequence_nr = HSR_SEQNR_START; - hsr->sup_sequence_nr = HSR_SUP_SEQNR_START; - hsr->interlink_sequence_nr = HSR_SEQNR_START; + atomic_set(&hsr->sequence_nr, HSR_SEQNR_START); + atomic_set(&hsr->sup_sequence_nr, HSR_SUP_SEQNR_START); timer_setup(&hsr->announce_timer, hsr_announce, 0); timer_setup(&hsr->prune_timer, hsr_prune_nodes, 0); diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c index b38060246e62e8a8d94d78d32a530e3820e752a1..6f63c8a775c41195c7cde5b27c83ffb91733d5c8 100644 --- a/net/hsr/hsr_forward.c +++ b/net/hsr/hsr_forward.c @@ -599,9 +599,7 @@ static void handle_std_frame(struct sk_buff *skb, if (port->type == HSR_PT_MASTER || port->type == HSR_PT_INTERLINK) { /* Sequence nr for the master/interlink node */ - lockdep_assert_held(&hsr->seqnr_lock); - frame->sequence_nr = hsr->sequence_nr; - hsr->sequence_nr++; + frame->sequence_nr = atomic_inc_return(&hsr->sequence_nr); } } diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h index ab1f8d35d9dcf507f0b2f90b0cede6dd24a51f2a..6f7bbf01f3e4f3033446860fb584402cb81df336 100644 --- a/net/hsr/hsr_main.h +++ b/net/hsr/hsr_main.h @@ -202,11 +202,9 @@ struct hsr_priv { struct timer_list prune_timer; struct timer_list prune_proxy_timer; int announce_count; - u16 sequence_nr; - u16 interlink_sequence_nr; /* Interlink port seq_nr */ - u16 sup_sequence_nr; /* For HSRv1 separate seq_nr for supervision */ + atomic_t sequence_nr; + atomic_t sup_sequence_nr; /* For HSRv1 separate seq_nr for supervision */ enum hsr_version prot_version; /* Indicate if HSRv0, HSRv1 or PRPv1 */ - spinlock_t seqnr_lock; /* locking for sequence_nr */ spinlock_t list_lock; /* locking for node list */ struct hsr_proto_ops *proto_ops; #define PRP_LAN_ID 0x5 /* 0x1010 for A and 0x1011 for B. Bit 0 is set diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index f6ff0b61e08a966254625ad13fd3b97e99329cf7..8aea4ff5f49e305555773e6e8cc7be570f946ada 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -163,7 +163,7 @@ static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev) if (nla_put(skb, IFLA_HSR_SUPERVISION_ADDR, ETH_ALEN, hsr->sup_multicast_addr) || - nla_put_u16(skb, IFLA_HSR_SEQ_NR, hsr->sequence_nr)) + nla_put_u16(skb, IFLA_HSR_SEQ_NR, atomic_read(&hsr->sequence_nr))) goto nla_put_failure; if (hsr->prot_version == PRP_V1) proto = HSR_PROTOCOL_PRP;
syzbot found a new splat [1]. Instead of adding yet another spin_lock_bh(&hsr->seqnr_lock) / spin_unlock_bh(&hsr->seqnr_lock) pair, remove seqnr_lock and use atomic_t for hsr->sequence_nr and hsr->sup_sequence_nr. This also avoid a race in hsr_fill_info(). Also remove interlink_sequence_nr which is unused. [1] WARNING: CPU: 1 PID: 9723 at net/hsr/hsr_forward.c:602 handle_std_frame+0x247/0x2c0 net/hsr/hsr_forward.c:602 Modules linked in: CPU: 1 UID: 0 PID: 9723 Comm: syz.0.1657 Not tainted 6.11.0-rc6-syzkaller-00026-g88fac17500f4 #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 RIP: 0010:handle_std_frame+0x247/0x2c0 net/hsr/hsr_forward.c:602 Code: 49 8d bd b0 01 00 00 be ff ff ff ff e8 e2 58 25 00 31 ff 89 c5 89 c6 e8 47 53 a8 f6 85 ed 0f 85 5a ff ff ff e8 fa 50 a8 f6 90 <0f> 0b 90 e9 4c ff ff ff e8 cc e7 06 f7 e9 8f fe ff ff e8 52 e8 06 RSP: 0018:ffffc90000598598 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffffc90000598670 RCX: ffffffff8ae2c919 RDX: ffff888024e94880 RSI: ffffffff8ae2c926 RDI: 0000000000000005 RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000003 R13: ffff8880627a8cc0 R14: 0000000000000000 R15: ffff888012b03c3a FS: 0000000000000000(0000) GS:ffff88802b700000(0063) knlGS:00000000f5696b40 CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033 CR2: 0000000020010000 CR3: 00000000768b4000 CR4: 0000000000350ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <IRQ> hsr_fill_frame_info+0x2c8/0x360 net/hsr/hsr_forward.c:630 fill_frame_info net/hsr/hsr_forward.c:700 [inline] hsr_forward_skb+0x7df/0x25c0 net/hsr/hsr_forward.c:715 hsr_handle_frame+0x603/0x850 net/hsr/hsr_slave.c:70 __netif_receive_skb_core.constprop.0+0xa3d/0x4330 net/core/dev.c:5555 __netif_receive_skb_list_core+0x357/0x950 net/core/dev.c:5737 __netif_receive_skb_list net/core/dev.c:5804 [inline] netif_receive_skb_list_internal+0x753/0xda0 net/core/dev.c:5896 gro_normal_list include/net/gro.h:515 [inline] gro_normal_list include/net/gro.h:511 [inline] napi_complete_done+0x23f/0x9a0 net/core/dev.c:6247 gro_cell_poll+0x162/0x210 net/core/gro_cells.c:66 __napi_poll.constprop.0+0xb7/0x550 net/core/dev.c:6772 napi_poll net/core/dev.c:6841 [inline] net_rx_action+0xa92/0x1010 net/core/dev.c:6963 handle_softirqs+0x216/0x8f0 kernel/softirq.c:554 do_softirq kernel/softirq.c:455 [inline] do_softirq+0xb2/0xf0 kernel/softirq.c:442 </IRQ> <TASK> Fixes: 06afd2c31d33 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.") Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- net/hsr/hsr_device.c | 35 ++++++++++------------------------- net/hsr/hsr_forward.c | 4 +--- net/hsr/hsr_main.h | 6 ++---- net/hsr/hsr_netlink.c | 2 +- 4 files changed, 14 insertions(+), 33 deletions(-)