Re: BUG: spinlock bad magic in tun_do_read

[Jason Wang]

On 2018å05æ09æ 10:50, Cong Wang wrote:
> On Mon, May 7, 2018 at 11:04 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> 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