Message ID | 20151218.155954.26632223306803039.davem@davemloft.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
> > Also you are at the point the tty is closing so the net device may be > > active. Don't you need to netif_stop_queue() or defer the buffer > > kfrees until after the network device is unregistered so you don't pee > > into free memory if you have a transmit occurring ? > > I'm pretty sure that's what the semaphore down above this sequence is > accomplishing. But if we do need the netif_stop_queue() let's do that > as a separate patch. Follow the code path for sp_xmit(). If sp_xmit is called it digs out sp from the ndetdev, locks sp->lock and stops the queue then calls sp_encaps which touches sp->xbuff. So if one thread of execution hits sp_xmit and another closes the ldisc at just the wrong moment then we have no protection. Alan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
==================== [PATCH] 6pack: Fix use after free in sixpack_close(). Need to do the unregister_device() after all references to the driver private have been done. Also we need to use del_timer_sync() for the timers so that we don't have any asynchronous references after the unregister. Signed-off-by: David S. Miller <davem@davemloft.net> --- drivers/net/hamradio/6pack.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c index 7c4a415..9f0b1c3 100644 --- a/drivers/net/hamradio/6pack.c +++ b/drivers/net/hamradio/6pack.c @@ -683,14 +683,14 @@ static void sixpack_close(struct tty_struct *tty) if (!atomic_dec_and_test(&sp->refcnt)) down(&sp->dead_sem); - unregister_netdev(sp->dev); - - del_timer(&sp->tx_t); - del_timer(&sp->resync_t); + del_timer_sync(&sp->tx_t); + del_timer_sync(&sp->resync_t); /* Free all 6pack frame buffers. */ kfree(sp->rbuff); kfree(sp->xbuff); + + unregister_netdev(sp->dev); } /* Perform I/O control on an active 6pack channel. */