[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a3276b9d-dd50-3f0f-d673-9cf93317f017@gmail.com>
Date: Mon, 14 Jan 2019 09:50:44 -0800
From: Eric Dumazet <eric.dumazet@...il.com>
To: Eric Dumazet <edumazet@...gle.com>, lorenzo.bianconi@...hat.com
Cc: netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: [PATCH net] gro_cell: add napi_disable in gro_cells_destroy
On 12/19/2018 02:46 PM, Eric Dumazet wrote:
> On Wed, Dec 19, 2018 at 2:23 PM Lorenzo Bianconi
> <lorenzo.bianconi@...hat.com> wrote:
>>
>> Add napi_disable routine in gro_cells_destroy since starting from
>> commit c42858eaf492 ("gro_cells: remove spinlock protecting receive
>> queues") gro_cell_poll and gro_cells_destroy can run concurrently on
>> napi_skbs list producing a kernel Oops if the tunnel interface is
>> removed while gro_cell_poll is running. The following Oops has been
>> triggered removing a vxlan device while the interface is receiving
>> traffic
>>
>
> This seems reasonable, I wonder why this bug has been hiding for so long...
>
> Acked-by: Eric Dumazet <edumazet@...gle.com>
>
Since we had another syzbot report involving this stuff [1], I took another look.
Apparently vxlan code does not look at (dev->flags & IFF_UP) before injecting
a packet (calling gro_cells_receive())
IP tunnels do this check properly.
[1]
HEAD commit: b71acb0e3721 Merge branch 'linus' of git://git.kernel.org/..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=14314bab400000
kernel config: https://syzkaller.appspot.com/x/.config?x=b03c5892bb940c76
dashboard link: https://syzkaller.appspot.com/bug?extid=6fe674089f9deb9f7726
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
I suspect the following fix is needed for vxlan
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 5209ee9aac47846367d7f469a7e69d08c030087e..7a443c251e604c41005d7d0f73832c22aed51768 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1657,6 +1657,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
goto drop;
}
+ if (unlikely(!(vxlan->dev->flags & IFF_UP))) {
+ atomic_long_inc(&vxlan->dev->rx_dropped);
+ goto drop;
+ }
stats = this_cpu_ptr(vxlan->dev->tstats);
u64_stats_update_begin(&stats->syncp);
stats->rx_packets++;
Powered by blists - more mailing lists