lkml.org 
[lkml]   [2010]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [98/223] TTY: open/hangup race fixup
On 12/13/2010 12:46 AM, Andi Kleen wrote:
> 2.6.35-longterm review patch. If anyone has any objections, please let me know.
>
> ------------------
> From: Jiri Slaby <jslaby@suse.cz>
>
> commit acfa747baf73922021a047f2d87a2d866f5dbab5 upstream.
>
> Like in the "TTY: don't allow reopen when ldisc is changing" patch,
> this one fixes a TTY WARNING as described in the option 1) there:
> 1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
> section tty_lock is held. However tty_lock is temporarily dropped in
> the middle of the function by tty_ldisc_hangup.
>
> The fix is to introduce a new flag which we set during the unlocked
> window and check it in tty_reopen too. The flag is TTY_HUPPING and is
> cleared after TTY_HUPPED is set.
>
> While at it, remove duplicate TTY_HUPPED set_bit. The one after
> calling ops->hangup seems to be more correct. But anyway, we hold
> tty_lock, so there should be no difference.
>
> Also document the function it does that kind of crap.
>
> Nicely reproducible with two forked children:
> static void do_work(const char *tty)
> {
> if (signal(SIGHUP, SIG_IGN) == SIG_ERR) exit(1);
> setsid();
> while (1) {
> int fd = open(tty, O_RDWR|O_NOCTTY);
> if (fd < 0) continue;
> if (ioctl(fd, TIOCSCTTY)) continue;
> if (vhangup()) continue;
> close(fd);
> }
> exit(0);
> }
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: <Valdis.Kletnieks@vt.edu>
> Reported-by: Kyle McMartin <kyle@mcmartin.ca>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
> drivers/char/tty_io.c | 10 +++++++++-
> include/linux/tty.h | 1 +
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> Index: linux/drivers/char/tty_io.c
> ===================================================================
> --- linux.orig/drivers/char/tty_io.c
> +++ linux/drivers/char/tty_io.c
> @@ -514,7 +514,10 @@ static void do_tty_hangup(struct work_st
> spin_unlock(&redirect_lock);
>
> /* inuse_filps is protected by the single kernel lock */
> +
> lock_kernel();
> + /* some functions below drop BTM, so we need this bit */
> + set_bit(TTY_HUPPING, &tty->flags);

This and 97/223 should not be needed in 2.6.35 -- there is BKL
protecting the race. IOW BKL removal in 2.6.36 introduced the regression
these 2 patches fix.

regards,
--
js
suse labs


\
 
 \ /
  Last update: 2010-12-13 10:29    [from the cache]
©2003-2011 Jasper Spaans