Re: [PATCH net-next] tuntap: fix possible deadlock when fail to register netdev

[Jason Wang]

On 2017å12æ08æ 11:11, Eric Dumazet wrote:
> On Fri, 2017-12-08 at 10:54 +0800, Jason Wang wrote:
> > Private destructor could be called when register_netdev() fail with
> > rtnl lock held. This will lead deadlock in tun_free_netdev() who
> > tries
> > to hold rtnl_lock. Fixing this by switching to use spinlock to
> > synchronize.
> >
> > Fixes: 96f84061620c ("tun: add eBPF based queue selection method")
> > Reported-by: Eric Dumazet <eric.dumazet@xxxxxxxxx>
> > Cc: Eric Dumazet <eric.dumazet@xxxxxxxxx>
> > Cc: Willem de Bruijn <willemb@xxxxxxxxxx>
> > Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
> > ---
> >  Âdrivers/net/tun.c | 7 ++++---
> >  Â1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 787cc35..f7ccd79 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -2050,8 +2050,11 @@ static int __tun_set_steering_ebpf(struct
> > tun_struct *tun,
> >  Â		new->prog = prog;
> >  Â	}
> >   
> > -	old = rtnl_dereference(tun->steering_prog);
> > +	spin_lock(&tun->lock);
> > +	old = rcu_dereference_protected(tun->steering_prog,
> > +					lock_is_held(&tun->lock));
> >  Â	rcu_assign_pointer(tun->steering_prog, new);
> > +	spin_unlock(&tun->lock);
> Hi Jason, thank you for the following up.
>
> Have you tested this code path with lockdep enabled ?

No I test without it.

> My gut feeling is that you need spin_lock_bh() here.
>
> Thanks

Yes, I miss the fact this the lock is used by e.g flow caches too. Will 
post V2.

Thanks