d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Close a race in the kern.ttys sysctl handler that resulted in panics in

Browse Source 
dev2udev() when a tty was being detached concurrently with the sysctl
handler:
- Hold the 'tty_list_mutex' lock while we read all the fields out of the
  struct tty for copying out later.  Previously the pty(4) and pts(4)
  destroy routines could set t_dev to NULL, drop their reference on the
  tty and destroy the cdev while the sysctl handler was attempting to
  invoke dev2udev() on the cdev being destroyed.  This happened when the
  sysctl handler read the value of t_dev prior to it being set to NULL
  either due to it being stale or due to timing races.  By holding the
  list lock we guarantee that the destroy routines will block in ttyrel()
  in that case and not destroy the cdev until after we've copied all of our
  data.  We may see a NULL cdev pointer or we may see the previous value,
  but the previous value will no longer point to a destroyed cdev if we
  see it.
- Fix the ttyfree() routine used by tty device drivers in their detach
  methods to use ttyrel() on the tty so we don't leak them.  Also, fix it
  to use the same order of operations as pty/pts destruction (set t_dev
  NULL, ttyrel(), destroy_dev()) so it cooperates with the sysctl handler.

MFC after:	3 days
Tested by:	avatar
This commit is contained in:
John Baldwin 2008-01-08 04:53:28 +00:00
parent 314464f422
commit 39033470fe
1 changed files with 20 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/kern/tty.c
View File

@ -3040,16 +3040,19 @@ ttygone(struct tty *tp)
*
* 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);
}
@ -3065,7 +3068,6 @@ sysctl_kern_ttys(SYSCTL_HANDLER_ARGS)
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;
@ -3074,6 +3076,18 @@ sysctl_kern_ttys(SYSCTL_HANDLER_ARGS)
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);
@ -3096,6 +3110,7 @@ sysctl_kern_ttys(SYSCTL_HANDLER_ARGS)
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);
@ -3108,7 +3123,9 @@ sysctl_kern_ttys(SYSCTL_HANDLER_ARGS)
mtx_unlock(&tty_list_mutex);
ttyrel(tp);
tp = tp2;
mtx_lock(&tty_list_mutex);
}
mtx_unlock(&tty_list_mutex);
return (0);
}