linux-kernel - Re: [PATCH] kdesu broken

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.01.0907281651160.3186@localhost.localdomain>
Date:	Tue, 28 Jul 2009 17:10:47 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Ray Lee <ray-lk@...rabbit.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] kdesu broken



On Wed, 29 Jul 2009, Alan Cox wrote:
> 
> BTW: The tty->low_latency fix doesn't work, because the ->write method
> can be called from an IRQ and that means we can't use ->low_latency=1 as
> we take mutexes.

Ok. So the end result is that Ogawa-san's fix is the right one. Then we 
can revert the low_latency=1 thing for pty's entirely. No?

And this is also the one that _looks_ the sanest - ie we do it on the read 
side, where it matters, rather than on the write side or the flush side 
(where the proper flushing can _also_ fix the problem, but where it's much 
more problematic, and where it's a lot less direct about what we care 
about).

This is just Ogawa's patch, redone against current -git, and with commit 
3a54297478e6578f96fd54bf4daa1751130aca86 reverted (Ogawa's patch already 
undid the non-low_latency part of it).

Now, I wonder if some _other_ line discipline might want to do that same 
tty_flush_to_ldisc thing in their "do I have data" logic, but I didn't 
look any closer.

So does this work for everyone? I haven't tested it yet myself, but this 
is the patch that "looks" right.

		Linus

---
 drivers/char/n_tty.c      |    1 +
 drivers/char/pty.c        |    2 --
 drivers/char/tty_buffer.c |   13 +++++++++++++
 include/linux/tty.h       |    1 +
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c
index ff47907..973be2f 100644
--- a/drivers/char/n_tty.c
+++ b/drivers/char/n_tty.c
@@ -1583,6 +1583,7 @@ static int n_tty_open(struct tty_struct *tty)
 
 static inline int input_available_p(struct tty_struct *tty, int amt)
 {
+	tty_flush_to_ldisc(tty);
 	if (tty->icanon) {
 		if (tty->canon_data)
 			return 1;
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index 3850a68..6e6942c 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -52,7 +52,6 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
 		return;
 	tty->link->packet = 0;
 	set_bit(TTY_OTHER_CLOSED, &tty->link->flags);
-	tty_flip_buffer_push(tty->link);
 	wake_up_interruptible(&tty->link->read_wait);
 	wake_up_interruptible(&tty->link->write_wait);
 	if (tty->driver->subtype == PTY_TYPE_MASTER) {
@@ -208,7 +207,6 @@ static int pty_open(struct tty_struct *tty, struct file *filp)
 	clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
 	set_bit(TTY_THROTTLED, &tty->flags);
 	retval = 0;
-	tty->low_latency = 1;
 out:
 	return retval;
 }
diff --git a/drivers/char/tty_buffer.c b/drivers/char/tty_buffer.c
index 810ee25..3108991 100644
--- a/drivers/char/tty_buffer.c
+++ b/drivers/char/tty_buffer.c
@@ -462,6 +462,19 @@ static void flush_to_ldisc(struct work_struct *work)
 }
 
 /**
+ *	tty_flush_to_ldisc
+ *	@tty: tty to push
+ *
+ *	Push the terminal flip buffers to the line discipline.
+ *
+ *	Must not be called from IRQ context.
+ */
+void tty_flush_to_ldisc(struct tty_struct *tty)
+{
+	flush_to_ldisc(&tty->buf.work.work);
+}
+
+/**
  *	tty_flip_buffer_push	-	terminal
  *	@tty: tty to push
  *
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1488d8c..e8c6c91 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -394,6 +394,7 @@ extern void __do_SAK(struct tty_struct *tty);
 extern void disassociate_ctty(int priv);
 extern void no_tty(void);
 extern void tty_flip_buffer_push(struct tty_struct *tty);
+extern void tty_flush_to_ldisc(struct tty_struct *tty);
 extern void tty_buffer_free_all(struct tty_struct *tty);
 extern void tty_buffer_flush(struct tty_struct *tty);
 extern void tty_buffer_init(struct tty_struct *tty);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/