kern/116837: ifconfig tunX destroy: panic

[KUROSAWA Takahiro]

The following reply was made to PR kern/116837; it has been noted by GNATS.

From: KUROSAWA Takahiro <fwkg7679 at mb.infoweb.ne.jp>
To: bug-followup at FreeBSD.org, jkpyvxmzsa at mailinator.com
Cc:  
Subject: Re: kern/116837: ifconfig tunX destroy: panic
Date: Sat, 12 Jan 2008 15:48:39 +0900

 The KASSERT() check in tun_destroy() seems incorrect
 since the function can actually be called while
 a user thread is opening /dev/tunX.  If we needed to
 ensure that no threads have fd for /dev/tunX in
 tun_destroy(), we should implement it in if_tun.
 
 Instead, we can rely on destroy_dev() to ensure that
 no threads access /dev/tunX anymore (the function
 blocks when there are threads accessing the device).
 But just deleting KASSERT() is insufficient because
 there is a race condition: tun_destroy() calls
 if_free() before destroy_dev(), so user threads might
 access the destroyed ifnet structure by read()/write()/...
 on /dev/tunX.
 
 I guess the following change is needed for if_tun.c:
 
 --- sys/net/if_tun.c	2008/01/11 04:14:11	1.1
 +++ sys/net/if_tun.c	2008/01/12 04:04:39
 @@ -249,15 +249,12 @@ tun_destroy(struct tun_softc *tp)
  {
  	struct cdev *dev;
  
 -	/* Unlocked read. */
 -	KASSERT((tp->tun_flags & TUN_OPEN) == 0,
 -	    ("tununits is out of sync - unit %d", TUN2IFP(tp)->if_dunit));
 -
  	dev = tp->tun_dev;
 +	/* destroy_dev() ensures no threads access /dev/tunX anymore. */
 +	destroy_dev(dev);
  	bpfdetach(TUN2IFP(tp));
  	if_detach(TUN2IFP(tp));
  	if_free(TUN2IFP(tp));
 -	destroy_dev(dev);
  	knlist_destroy(&tp->tun_rsel.si_note);
  	mtx_destroy(&tp->tun_mtx);
  	free(tp, M_TUN);