netdev - Re: [PATCH net] net: tun: do not call napi

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:   Wed, 29 Jun 2022 09:27:05 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        syzbot <syzkaller@...glegroups.com>,
        Petar Penkov <ppenkov@...atrix.com>
Subject: Re: [PATCH net] net: tun: do not call napi_disable() twice

On Wed, 29 Jun 2022 18:19:58 +0200 Eric Dumazet wrote:
> On Wed, Jun 29, 2022 at 6:17 PM Jakub Kicinski <kuba@...nel.org> wrote:
> >
> > On Wed, 29 Jun 2022 09:37:52 +0000 Eric Dumazet wrote:  
> > > syzbot reported a hang in tun_napi_disable() while RTNL is held.
> > >
> > > Because tun.c logic is complicated, I chose to:
> > >
> > > 1) rename tun->napi_enabled to tun->napi_configured
> > >
> > > 2) Add a new boolean, tracking if tun->napi is enabled or not.  
> >
> > Not a huge surprise TBH :S
> >
> > Is there a repro?  
> 
> Yes, here it is:
> 
> // autogenerated by syzkaller (https://github.com/google/syzkaller)

Thanks! let me test this:

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e2eb35887394..8776a9e1a8f5 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -661,7 +661,6 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
                        sock_put(&tfile->sk);
                } else {
                        tun_disable_queue(tun, tfile);
-                       tun_napi_disable(tfile);
                }
 
                synchronize_net();
@@ -719,6 +718,7 @@ static void tun_detach_all(struct net_device *dev)
                --tun->numqueues;
        }
        list_for_each_entry(tfile, &tun->disabled, next) {
+               tun_napi_disable(tfile);
                tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
                tfile->socket.sk->sk_data_ready(tfile->socket.sk);
                RCU_INIT_POINTER(tfile->tun, NULL);