lkml.org 
[lkml]   [2010]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/1] TTY: don't allow reopen when ldisc is changing
On 11/27/2010 10:43 AM, Jiri Slaby wrote:
> On 11/27/2010 09:50 AM, Jiri Slaby wrote:
>> On 11/27/2010 03:59 AM, Kyle McMartin wrote:
>>> I'm poking around to see, I think maybe something might be dropping
>>> locks in the callchain that gives us a window where this might be
>>> possible...
>>
>> Of course, that's the case:
>> clear_bit(TTY_LDISC, &tty->flags);
>> tty_unlock();
>> cancel_delayed_work_sync(&tty->buf.work);
>> mutex_unlock(&tty->ldisc_mutex);
>>
>> tty_lock();
>> mutex_lock(&tty->ldisc_mutex);
>>
>> in tty_ldisc_hangup. Hence my point 1) from previous posts doesn't hold too:
>> 1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
>> section tty_lock is held.
>>
>> I will check, how to fix this.
>
> Reproducible with 2 running processes from the attachment.

Is it fixed with the attached proof-of-concept patch?

So you need:
THIS ONE
TTY: don't allow reopen when ldisc is changing
TTY: ldisc, fix open flag handling
Char: TTY, restore tty_ldisc_wait_idle

The last one is in 2.6.37-rc2 already.

thanks,
--
js
suse labs
From 9e88e8b9915b5e067507a087437d80e6a133d612 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Sat, 27 Nov 2010 16:06:46 +0100
Subject: [PATCH 1/1] TTY: open/hangup race fixup


Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/tty/tty_io.c | 10 +++++++++-
include/linux/tty.h | 1 +
2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 878f6d6..35480dd 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -559,6 +559,9 @@ void __tty_hangup(struct tty_struct *tty)

tty_lock();

+ /* some functions below drop BTM, so we need this bit */
+ set_bit(TTY_HUPPING, &tty->flags);
+
/* inuse_filps is protected by the single tty lock,
this really needs to change if we want to flush the
workqueue with the lock held */
@@ -578,6 +581,10 @@ void __tty_hangup(struct tty_struct *tty)
}
spin_unlock(&tty_files_lock);

+ /*
+ * it drops BTM and thus races with reopen
+ * we protect the race by TTY_HUPPING
+ */
tty_ldisc_hangup(tty);

read_lock(&tasklist_lock);
@@ -615,7 +622,6 @@ void __tty_hangup(struct tty_struct *tty)
tty->session = NULL;
tty->pgrp = NULL;
tty->ctrl_status = 0;
- set_bit(TTY_HUPPED, &tty->flags);
spin_unlock_irqrestore(&tty->ctrl_lock, flags);

/* Account for the p->signal references we killed */
@@ -641,6 +647,7 @@ void __tty_hangup(struct tty_struct *tty)
* can't yet guarantee all that.
*/
set_bit(TTY_HUPPED, &tty->flags);
+ clear_bit(TTY_HUPPING, &tty->flags);
tty_ldisc_enable(tty);

tty_unlock();
@@ -1311,6 +1318,7 @@ static int tty_reopen(struct tty_struct *tty)
struct tty_driver *driver = tty->driver;

if (test_bit(TTY_CLOSING, &tty->flags) ||
+ test_bit(TTY_HUPPING, &tty->flags) ||
test_bit(TTY_LDISC_CHANGING, &tty->flags))
return -EIO;

diff --git a/include/linux/tty.h b/include/linux/tty.h
index 032d79f..54e4eaa 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -366,6 +366,7 @@ struct tty_file_private {
#define TTY_HUPPED 18 /* Post driver->hangup() */
#define TTY_FLUSHING 19 /* Flushing to ldisc in progress */
#define TTY_FLUSHPENDING 20 /* Queued buffer flush pending */
+#define TTY_HUPPING 21 /* ->hangup() in progress */

#define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))

--
1.7.3.1
\
 
 \ /
  Last update: 2010-11-27 16:13    [W:0.045 / U:1.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site