Re: [PATCH net] gro_cell: add napi_disable in gro_cells_destroy

[Eric Dumazet]

On 12/19/2018 02:46 PM, Eric Dumazet wrote:
> On Wed, Dec 19, 2018 at 2:23 PM Lorenzo Bianconi
> <lorenzo.bianc...@redhat.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 <eduma...@google.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++;