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
| ||
|
Date: Wed, 9 May 2018 11:38:11 +0800 From: Jason Wang <jasowang@...hat.com> To: Cong Wang <xiyou.wangcong@...il.com>, Eric Dumazet <eric.dumazet@...il.com> Cc: syzbot <syzbot+e8b902c3c3fadf0a9dba@...kaller.appspotmail.com>, David Miller <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, LKML <linux-kernel@...r.kernel.org>, "Michael S. Tsirkin" <mst@...hat.com>, Linux Kernel Network Developers <netdev@...r.kernel.org>, Petar Penkov <peterpenkov96@...il.com>, Sabrina Dubroca <sd@...asysnail.net>, syzkaller-bugs@...glegroups.com Subject: Re: BUG: spinlock bad magic in tun_do_read On 2018年05月09日 10:50, Cong Wang wrote: > On Mon, May 7, 2018 at 11:04 PM, Eric Dumazet <eric.dumazet@...il.com> wrote: >> >> On 05/07/2018 10:54 PM, Cong Wang wrote: >>> Yeah, we should return early before hitting this uninitialized ptr ring... >>> Something like: >>> >>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >>> index ef33950a45d9..638c87a95247 100644 >>> --- a/drivers/net/tun.c >>> +++ b/drivers/net/tun.c >>> @@ -2128,6 +2128,9 @@ static void *tun_ring_recv(struct tun_file >>> *tfile, int noblock, int *err) >>> void *ptr = NULL; >>> int error = 0; >>> >>> + if (!tfile->tx_ring.queue) >>> + goto out; >>> + >>> >>> Or, checking if tun is detached... >>> >>> >> tx_ring was properly initialized when first ptr_ring_consume() at line 2131 was attempted. >> >> The bug happens later at line 2143 , after a schedule() call, line 2155 >> >> So a single check at function prologue wont solve the case the thread had to sleep, >> then some uninit happened. > > Very good point. RTNL lock is supposed to protect cleanup path, but I don't > think we can acquire RTNL for tun_chr_read_iter() path... I think the root cause is we try to initialize ptr ring during TUNSETIFF since the length depends on the dev->tx_queue_len and try to destroy it when device is gone. We can solve this by initializing a zero size ptr_ring during open() and resize if necessary. Then there no need for any workaround like memset and checking against NULL. Let me try to cook a patch for this. Thanks
Powered by blists - more mailing lists