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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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