netdev - Re: [PATCH net] gro_cell: add napi_disable in gro_cells

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

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++;