Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Jun 2006 17:03:52 +0100
From:      Gavin Atkinson <gavin.atkinson@ury.york.ac.uk>
To:        Martin Blapp <mb@imp.ch>
Cc:        Robert Watson <rwatson@freebsd.org>, Patrick Guelat <patg@imp.ch>, "Wojciech A. Koszek" <wkoszek@freebsd.org>, freebsd-stable@freebsd.org
Subject:   Re: Crash with FreeBSD 6.1 STABLE of today
Message-ID:  <1151078632.62769.30.camel@buffy.york.ac.uk>
In-Reply-To: <20060623133915.S14714@godot.imp.ch>
References:  <20060621202508.S17514@godot.imp.ch> <20060621193941.Y8526@fledge.watson.org> <20060622205806.GA6542@FreeBSD.czest.pl> <20060622223630.V17514@godot.imp.ch> <1151056731.62769.2.camel@buffy.york.ac.uk> <20060623133915.S14714@godot.imp.ch>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 2006-06-23 at 13:46 +0200, Martin Blapp wrote:
> Hi,
> 
> Maybe this is the solution ? IMHO there is a race window
> open between the first tp->t_session test and the locking
> of the proc tree.

I'm not sure if t_session is supposed to be protected by the proctree
lock though.  With an initial glance of the code, it would seem odd to
be protected by the proctree lock, although I can't see any other locks
Someone with more knowledge of this code will probably know the answer
to this.  

There does seem to be a worrying comment above tty_close (which is the
only place that t_session seems to be set to NULL):

 * XXX our caller should have done `spltty(); l_close(); tty_close();'
 * and l_close() should have flushed, but we repeat the spltty() and
 * the flush in case there are buggy callers.

As I understand it, spltty() is now a no-op.  Does this mean that this
code is now essentially running without any locks that were used to
serialise changes to struct tty in days gone by?  Or is the whole tty
subsystem still running under Giant?

Gavin

> 
> +++ src/sys/kern/tty.c
> --- src/sys/kern/tty.c
> 
> +                       sx_slock(&proctree_lock);
>                          if (tp->t_session) {
> -				sx_slock(&proctree_lock);
>                                  if (tp->t_session->s_leader) {
>                                          struct proc *p;
> 
>                                          p = tp->t_session->s_leader;
>                                          PROC_LOCK(p);
>                                          psignal(p, SIGHUP);
>                                          PROC_UNLOCK(p);
>                                  }
> -				sx_sunlock(&proctree_lock);
>                          }
> +                       sx_sunlock(&proctree_lock);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1151078632.62769.30.camel>