Re: [Fwd: Re: kern/118258 sysctl causing panics on 7.0-xxx]

On Thursday 29 November 2007 10:53:32 pm Tai-hwa Liang wrote:
> On Wed, 28 Nov 2007, Remko Lodder wrote:
> > Hello,
> >
> > So as per Jeff's information, can someone from the -current
> > list either contact jeff or try to resolve the problems
> > mentioned? :)
> 
>    This is a longstanding bug which also exists in RELENG_6.  It turns out
> that 'sysctl kern.ttys' after a terminal device is removed could trigger
> this panic reliably.  For example, do 'sysctl kern.ttys' multiple times
> after detaching an USB serial-to-rs232 cable or a PCMCIA modem card.
> 
>    Alternatively, following script would demo the panic if you don't have
> a physically removable terminal device:
> 
> #!/bin/sh
> #
> # Warning! Running this script as root will panic your CURRENT box...
> #
> while true; do
>  	kldload dcons
>  	kldunload dcons
>  	ls /dev
>  	sysctl kern.ttys
>  	sleep 1
> done
> 
>    This seems to be a race between devfs and destroy_dev(), Cc'ing kib_at_
> since he probably has more clues in this area.

Try this patch.  Also available at
http://www.FreeBSD.org/~jhb/patches/ttys_sysctl.patch

--- //depot/vendor/freebsd/src/sys/fs/devfs/devfs_vnops.c	2007/10/24 19:06:35
+++ //depot/user/jhb/acpipci/fs/devfs/devfs_vnops.c	2007/11/01 17:09:40
_at__at_ -995,17 +995,20 _at__at_
 
 	vnode_destroy_vobject(vp);
 
+	VI_LOCK(vp);
 	dev_lock();
 	dev = vp->v_rdev;
 	vp->v_rdev = NULL;
 
 	if (dev == NULL) {
 		dev_unlock();
+		VI_UNLOCK(vp);
 		return (0);
 	}
 
 	dev->si_usecount -= vp->v_usecount;
 	dev_unlock();
+	VI_UNLOCK(vp);
 	dev_rel(dev);
 	return (0);
 }
--- //depot/vendor/freebsd/src/sys/kern/tty.c	2007/07/20 09:45:18
+++ //depot/user/jhb/acpipci/kern/tty.c	2007/11/13 18:59:58
_at__at_ -3040,16 +3040,19 _at__at_
  *
  * XXX: This shall sleep until all threads have left the driver.
  */
- 
 void
 ttyfree(struct tty *tp)
 {
+	struct cdev *dev;
 	u_int unit;
  
 	mtx_assert(&Giant, MA_OWNED);
 	ttygone(tp);
 	unit = tp->t_devunit;
-	destroy_dev(tp->t_mdev);
+	dev = tp->t_mdev;
+	tp->t_dev = NULL;
+	ttyrel(tp);
+	destroy_dev(dev);
 	free_unr(tty_unit, unit);
 }
 
_at__at_ -3065,7 +3068,6 _at__at_
 	tp = TAILQ_FIRST(&tty_list);
 	if (tp != NULL)
 		ttyref(tp);
-	mtx_unlock(&tty_list_mutex);
 	while (tp != NULL) {
 		bzero(&xt, sizeof xt);
 		xt.xt_size = sizeof xt;
_at__at_ -3074,6 +3076,18 _at__at_
 		xt.xt_cancc = tp->t_canq.c_cc;
 		xt.xt_outcc = tp->t_outq.c_cc;
 		XT_COPY(line);
+
+		/*
+		 * XXX: We hold the tty list lock while doing this to
+		 * work around a race with pty/pts tty destruction.
+		 * They set t_dev to NULL and then call ttyrel() to
+		 * free the structure which will block on the list
+		 * lock before they call destroy_dev() on the cdev
+		 * backing t_dev.
+		 *
+		 * XXX: ttyfree() now does the same since it has been
+		 * fixed to not leak ttys.
+		 */
 		if (tp->t_dev != NULL)
 			xt.xt_dev = dev2udev(tp->t_dev);
 		XT_COPY(state);
_at__at_ -3096,6 +3110,7 _at__at_
 		XT_COPY(olowat);
 		XT_COPY(ospeedwat);
 #undef XT_COPY
+		mtx_unlock(&tty_list_mutex);
 		error = SYSCTL_OUT(req, &xt, sizeof xt);
 		if (error != 0) {
 			ttyrel(tp);
_at__at_ -3108,6 +3123,7 _at__at_
 		mtx_unlock(&tty_list_mutex);
 		ttyrel(tp);
 		tp = tp2;
+		mtx_lock(&tty_list_mutex);
 	}
 	return (0);
 }
 

-- 
John Baldwin