| Message ID | 20220629181911.372047-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ff1fa2081d173b01cebe2fbf0a2d0f1cee9ce4b5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] net: tun: avoid disabling NAPI twice | expand |
On Wed, Jun 29, 2022 at 8:19 PM Jakub Kicinski <kuba@kernel.org> wrote: > > Eric reports that syzbot made short work out of my speculative > fix. Indeed when queue gets detached its tfile->tun remains, > so we would try to stop NAPI twice with a detach(), close() > sequence. > > Alternative fix would be to move tun_napi_disable() to > tun_detach_all() and let the NAPI run after the queue > has been detached. > > Fixes: a8fc8cb5692a ("net: tun: stop NAPI when detaching queues") Reviewed-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> > Reported-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > Cc: Petar Penkov <ppenkov@aviatrix.com> > --- > CC: ppenkov@aviatrix.com > --- > drivers/net/tun.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index e2eb35887394..259b2b84b2b3 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -640,7 +640,8 @@ static void __tun_detach(struct tun_file *tfile, bool clean) > tun = rtnl_dereference(tfile->tun); > > if (tun && clean) { > - tun_napi_disable(tfile); > + if (!tfile->detached) > + tun_napi_disable(tfile); > tun_napi_del(tfile); > } > > -- > 2.36.1 >
Hello: This series was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Wed, 29 Jun 2022 11:19:10 -0700 you wrote: > Eric reports that syzbot made short work out of my speculative > fix. Indeed when queue gets detached its tfile->tun remains, > so we would try to stop NAPI twice with a detach(), close() > sequence. > > Alternative fix would be to move tun_napi_disable() to > tun_detach_all() and let the NAPI run after the queue > has been detached. > > [...] Here is the summary with links: - [net,1/2] net: tun: avoid disabling NAPI twice https://git.kernel.org/netdev/net/c/ff1fa2081d17 - [net,2/2] selftest: tun: add test for NAPI dismantle https://git.kernel.org/netdev/net/c/839b92fede7b You are awesome, thank you!
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index e2eb35887394..259b2b84b2b3 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -640,7 +640,8 @@ static void __tun_detach(struct tun_file *tfile, bool clean) tun = rtnl_dereference(tfile->tun); if (tun && clean) { - tun_napi_disable(tfile); + if (!tfile->detached) + tun_napi_disable(tfile); tun_napi_del(tfile); }
Eric reports that syzbot made short work out of my speculative fix. Indeed when queue gets detached its tfile->tun remains, so we would try to stop NAPI twice with a detach(), close() sequence. Alternative fix would be to move tun_napi_disable() to tun_detach_all() and let the NAPI run after the queue has been detached. Fixes: a8fc8cb5692a ("net: tun: stop NAPI when detaching queues") Reported-by: syzbot <syzkaller@googlegroups.com> Reported-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- Cc: Petar Penkov <ppenkov@aviatrix.com> --- CC: ppenkov@aviatrix.com --- drivers/net/tun.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)