Re: Re: [PATCH] drivers: net: slip: fix NPD bug in sl_tx_timeout()

[duoming]

Hello,

On Wed, 6 Apr 2022 10:45:05 +0200, Jiri Slaby wrote:

> > When a slip driver is detaching, the slip_close() will act to
> > cleanup necessary resources and sl->tty is set to NULL in
> > slip_close(). Meanwhile, the packet we transmit is blocked,
> > sl_tx_timeout() will be called. Although slip_close() and
> > sl_tx_timeout() use sl->lock to synchronize, we don`t judge
> > whether sl->tty equals to NULL in sl_tx_timeout() and the
> > null pointer dereference bug will happen.
> > 
> >     (Thread 1)                 |      (Thread 2)
> >                                | slip_close()
> >                                |   spin_lock_bh(&sl->lock)
> >                                |   ...
> > ...                           |   sl->tty = NULL //(1)
> > sl_tx_timeout()               |   spin_unlock_bh(&sl->lock)
> >    spin_lock(&sl->lock);       |
> >    ...                         |   ...
> >    tty_chars_in_buffer(sl->tty)|
> >      if (tty->ops->..) //(2)   |
> >      ...                       |   synchronize_rcu()
> > 
> > We set NULL to sl->tty in position (1) and dereference sl->tty
> > in position (2).
> > 
> > This patch adds check in sl_tx_timeout(). If sl->tty equals to
> > NULL, sl_tx_timeout() will goto out.
> 
> It makes sense. unregister_netdev() (to kill the timer) is called only 
> later in slip_close().
> 
> Reviewed-by: Jiri Slaby <jirislaby@xxxxxxxxxx>
> 

Thanks for your time and approve my patch.

Best regards,
Duoming Zhou