mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Matthias Urlichs <matthias@urlichs.de>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ldisc switching on a HUPped pty hangs the caller
Date: Fri, 15 Jan 2010 10:48:34 +0000	[thread overview]
Message-ID: <20100115104834.4765b584@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <1263532072.22171.24.camel@kiste>

> I recently upgraded my gateway machine to 2.6.31. This caused
> ppp-over-ssh to stop working. Indeed, the PPP process got wedged into
> noninterruptible sleep, which this patch fixes.
> 
> (The comment, by the way, seems to be wrong. There was no race.) 

Really ?

Think about

					set_ldisc
	hangup
	close
	open
	set to N_TTY etc
					Now what ???

 
> The underlying problem, however, turns out to be the vhangup() syscall
> which the SSH server emits before exec'ing pppd. This causes the tty's
> HUPPED bit to get set, which in turn causes the above error.

vhangup sets the huppet bit to make sure that anything touching the tty
beyond that point goes away and dies.

> diff --git a/drivers/char/tty_ldisc.c b/drivers/char/tty_ldisc.c
> index aafdbae..bb92f5e 100644
> --- a/drivers/char/tty_ldisc.c
> +++ b/drivers/char/tty_ldisc.c
> @@ -621,9 +621,8 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
>  		/* We were raced by the hangup method. It will have stomped
>  		   the ldisc data and closed the ldisc down */
>  		clear_bit(TTY_LDISC_CHANGING, &tty->flags);
> -		mutex_unlock(&tty->ldisc_mutex);
> -		tty_ldisc_put(new_ldisc);
> -		return -EIO;
> +		retval = -EIO;
> +		goto out;
>  	}
>  
>  	/* Shutdown the current discipline. */
> @@ -652,7 +651,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
>  	/*
>  	 *	Allow ldisc referencing to occur again
>  	 */
> -
> +out:
>  	tty_ldisc_enable(tty);
>  	if (o_tty)
>  		tty_ldisc_enable(o_tty);

And falls through to restart work queues and stuff that may not be safe
to do


So: NAK

I agree there is a bug but you've swapped one bug for sevral nastier bugs.

As far as  I can see from a quick look the real problem in your case is
that we don't do enough work in the case where tty->driver->flags doesn't
contain TTY_DRIVER_RESET_TERMIOS. We need to reinit the ldisc either way.


  reply	other threads:[~2010-01-15 10:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-15  5:07 Matthias Urlichs
2010-01-15 10:48 ` Alan Cox [this message]
2010-01-15 12:04   ` Matthias Urlichs
2010-01-15 11:16 ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100115104834.4765b584@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthias@urlichs.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®