Commit c65c9bc3 authored by Alan Cox's avatar Alan Cox Committed by Linus Torvalds

tty: rewrite the ldisc locking

There are several pretty much unfixable races in the old ldisc code, especially
with respect to pty behaviour and also to hangup. It's easier to rewrite the
code than simply try and patch it up.

This patch
- splits the ldisc from the tty (so we will be able to refcount it more cleanly
  later)
- introduces a mutex lock for ldisc changing on an active device
- fixes the complete mess that hangup caused
- implements hopefully correct setldisc/close/hangup locking

There are still some problems around pty pairs that have always been there but
at least it is now possible to understand the code and fix further problems.

This fixes the following known bugs
- hang up can leak ldisc references
- hang up may not call open/close on ldisc in a matched way
- pty/tty pairs can deadlock during an ldisc change
- reading the ldisc proc files can cause every ldisc to be loaded

and probably a few other of the mysterious ldisc race reports.

I'm sure it also adds the odd new one.
Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent e8b70e7d
...@@ -277,8 +277,8 @@ static int hci_uart_tty_open(struct tty_struct *tty) ...@@ -277,8 +277,8 @@ static int hci_uart_tty_open(struct tty_struct *tty)
/* FIXME: why is this needed. Note don't use ldisc_ref here as the /* FIXME: why is this needed. Note don't use ldisc_ref here as the
open path is before the ldisc is referencable */ open path is before the ldisc is referencable */
if (tty->ldisc.ops->flush_buffer) if (tty->ldisc->ops->flush_buffer)
tty->ldisc.ops->flush_buffer(tty); tty->ldisc->ops->flush_buffer(tty);
tty_driver_flush_buffer(tty); tty_driver_flush_buffer(tty);
return 0; return 0;
......
...@@ -5200,7 +5200,7 @@ static int cyclades_proc_show(struct seq_file *m, void *v) ...@@ -5200,7 +5200,7 @@ static int cyclades_proc_show(struct seq_file *m, void *v)
(cur_jifs - info->idle_stats.recv_idle)/ (cur_jifs - info->idle_stats.recv_idle)/
HZ, info->idle_stats.overruns, HZ, info->idle_stats.overruns,
/* FIXME: double check locking */ /* FIXME: double check locking */
(long)info->port.tty->ldisc.ops->num); (long)info->port.tty->ldisc->ops->num);
else else
seq_printf(m, "%3d %8lu %10lu %8lu " seq_printf(m, "%3d %8lu %10lu %8lu "
"%10lu %8lu %9lu %6ld\n", "%10lu %8lu %9lu %6ld\n",
......
...@@ -2114,8 +2114,8 @@ static int pc_ioctl(struct tty_struct *tty, struct file *file, ...@@ -2114,8 +2114,8 @@ static int pc_ioctl(struct tty_struct *tty, struct file *file,
tty_wait_until_sent(tty, 0); tty_wait_until_sent(tty, 0);
} else { } else {
/* ldisc lock already held in ioctl */ /* ldisc lock already held in ioctl */
if (tty->ldisc.ops->flush_buffer) if (tty->ldisc->ops->flush_buffer)
tty->ldisc.ops->flush_buffer(tty); tty->ldisc->ops->flush_buffer(tty);
} }
unlock_kernel(); unlock_kernel();
/* Fall Thru */ /* Fall Thru */
......
...@@ -868,11 +868,11 @@ i2Input(i2ChanStrPtr pCh) ...@@ -868,11 +868,11 @@ i2Input(i2ChanStrPtr pCh)
amountToMove = count; amountToMove = count;
} }
// Move the first block // Move the first block
pCh->pTTY->ldisc.ops->receive_buf( pCh->pTTY, pCh->pTTY->ldisc->ops->receive_buf( pCh->pTTY,
&(pCh->Ibuf[stripIndex]), NULL, amountToMove ); &(pCh->Ibuf[stripIndex]), NULL, amountToMove );
// If we needed to wrap, do the second data move // If we needed to wrap, do the second data move
if (count > amountToMove) { if (count > amountToMove) {
pCh->pTTY->ldisc.ops->receive_buf( pCh->pTTY, pCh->pTTY->ldisc->ops->receive_buf( pCh->pTTY,
pCh->Ibuf, NULL, count - amountToMove ); pCh->Ibuf, NULL, count - amountToMove );
} }
// Bump and wrap the stripIndex all at once by the amount of data read. This // Bump and wrap the stripIndex all at once by the amount of data read. This
......
...@@ -1315,8 +1315,8 @@ static inline void isig(int sig, struct tty_struct *tty, int flush) ...@@ -1315,8 +1315,8 @@ static inline void isig(int sig, struct tty_struct *tty, int flush)
if (tty->pgrp) if (tty->pgrp)
kill_pgrp(tty->pgrp, sig, 1); kill_pgrp(tty->pgrp, sig, 1);
if (flush || !L_NOFLSH(tty)) { if (flush || !L_NOFLSH(tty)) {
if ( tty->ldisc.ops->flush_buffer ) if ( tty->ldisc->ops->flush_buffer )
tty->ldisc.ops->flush_buffer(tty); tty->ldisc->ops->flush_buffer(tty);
i2InputFlush( tty->driver_data ); i2InputFlush( tty->driver_data );
} }
} }
......
...@@ -342,8 +342,8 @@ static int n_hdlc_tty_open (struct tty_struct *tty) ...@@ -342,8 +342,8 @@ static int n_hdlc_tty_open (struct tty_struct *tty)
#endif #endif
/* Flush any pending characters in the driver and discipline. */ /* Flush any pending characters in the driver and discipline. */
if (tty->ldisc.ops->flush_buffer) if (tty->ldisc->ops->flush_buffer)
tty->ldisc.ops->flush_buffer(tty); tty->ldisc->ops->flush_buffer(tty);
tty_driver_flush_buffer(tty); tty_driver_flush_buffer(tty);
......
...@@ -110,7 +110,7 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, ...@@ -110,7 +110,7 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf,
c = to->receive_room; c = to->receive_room;
if (c > count) if (c > count)
c = count; c = count;
to->ldisc.ops->receive_buf(to, buf, NULL, c); to->ldisc->ops->receive_buf(to, buf, NULL, c);
return c; return c;
} }
...@@ -148,11 +148,11 @@ static int pty_chars_in_buffer(struct tty_struct *tty) ...@@ -148,11 +148,11 @@ static int pty_chars_in_buffer(struct tty_struct *tty)
int count; int count;
/* We should get the line discipline lock for "tty->link" */ /* We should get the line discipline lock for "tty->link" */
if (!to || !to->ldisc.ops->chars_in_buffer) if (!to || !to->ldisc->ops->chars_in_buffer)
return 0; return 0;
/* The ldisc must report 0 if no characters available to be read */ /* The ldisc must report 0 if no characters available to be read */
count = to->ldisc.ops->chars_in_buffer(to); count = to->ldisc->ops->chars_in_buffer(to);
if (tty->driver->subtype == PTY_TYPE_SLAVE) if (tty->driver->subtype == PTY_TYPE_SLAVE)
return count; return count;
...@@ -186,8 +186,8 @@ static void pty_flush_buffer(struct tty_struct *tty) ...@@ -186,8 +186,8 @@ static void pty_flush_buffer(struct tty_struct *tty)
if (!to) if (!to)
return; return;
if (to->ldisc.ops->flush_buffer) if (to->ldisc->ops->flush_buffer)
to->ldisc.ops->flush_buffer(to); to->ldisc->ops->flush_buffer(to);
if (to->packet) { if (to->packet) {
spin_lock_irqsave(&tty->ctrl_lock, flags); spin_lock_irqsave(&tty->ctrl_lock, flags);
......
...@@ -327,7 +327,7 @@ int paste_selection(struct tty_struct *tty) ...@@ -327,7 +327,7 @@ int paste_selection(struct tty_struct *tty)
} }
count = sel_buffer_lth - pasted; count = sel_buffer_lth - pasted;
count = min(count, tty->receive_room); count = min(count, tty->receive_room);
tty->ldisc.ops->receive_buf(tty, sel_buffer + pasted, tty->ldisc->ops->receive_buf(tty, sel_buffer + pasted,
NULL, count); NULL, count);
pasted += count; pasted += count;
} }
......
...@@ -491,22 +491,6 @@ void tty_ldisc_flush(struct tty_struct *tty) ...@@ -491,22 +491,6 @@ void tty_ldisc_flush(struct tty_struct *tty)
EXPORT_SYMBOL_GPL(tty_ldisc_flush); EXPORT_SYMBOL_GPL(tty_ldisc_flush);
/**
* tty_reset_termios - reset terminal state
* @tty: tty to reset
*
* Restore a terminal to the driver default state
*/
static void tty_reset_termios(struct tty_struct *tty)
{
mutex_lock(&tty->termios_mutex);
*tty->termios = tty->driver->init_termios;
tty->termios->c_ispeed = tty_termios_input_baud_rate(tty->termios);
tty->termios->c_ospeed = tty_termios_baud_rate(tty->termios);
mutex_unlock(&tty->termios_mutex);
}
/** /**
* do_tty_hangup - actual handler for hangup events * do_tty_hangup - actual handler for hangup events
* @work: tty device * @work: tty device
...@@ -536,7 +520,6 @@ static void do_tty_hangup(struct work_struct *work) ...@@ -536,7 +520,6 @@ static void do_tty_hangup(struct work_struct *work)
struct file *cons_filp = NULL; struct file *cons_filp = NULL;
struct file *filp, *f = NULL; struct file *filp, *f = NULL;
struct task_struct *p; struct task_struct *p;
struct tty_ldisc *ld;
int closecount = 0, n; int closecount = 0, n;
unsigned long flags; unsigned long flags;
int refs = 0; int refs = 0;
...@@ -567,40 +550,8 @@ static void do_tty_hangup(struct work_struct *work) ...@@ -567,40 +550,8 @@ static void do_tty_hangup(struct work_struct *work)
filp->f_op = &hung_up_tty_fops; filp->f_op = &hung_up_tty_fops;
} }
file_list_unlock(); file_list_unlock();
/*
* FIXME! What are the locking issues here? This may me overdoing
* things... This question is especially important now that we've
* removed the irqlock.
*/
ld = tty_ldisc_ref(tty);
if (ld != NULL) {
/* We may have no line discipline at this point */
if (ld->ops->flush_buffer)
ld->ops->flush_buffer(tty);
tty_driver_flush_buffer(tty);
if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) &&
ld->ops->write_wakeup)
ld->ops->write_wakeup(tty);
if (ld->ops->hangup)
ld->ops->hangup(tty);
}
/*
* FIXME: Once we trust the LDISC code better we can wait here for
* ldisc completion and fix the driver call race
*/
wake_up_interruptible_poll(&tty->write_wait, POLLOUT);
wake_up_interruptible_poll(&tty->read_wait, POLLIN);
/*
* Shutdown the current line discipline, and reset it to
* N_TTY.
*/
if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS)
tty_reset_termios(tty);
/* Defer ldisc switch */
/* tty_deferred_ldisc_switch(N_TTY);
This should get done automatically when the port closes and tty_ldisc_hangup(tty);
tty_release is called */
read_lock(&tasklist_lock); read_lock(&tasklist_lock);
if (tty->session) { if (tty->session) {
...@@ -629,12 +580,15 @@ static void do_tty_hangup(struct work_struct *work) ...@@ -629,12 +580,15 @@ static void do_tty_hangup(struct work_struct *work)
read_unlock(&tasklist_lock); read_unlock(&tasklist_lock);
spin_lock_irqsave(&tty->ctrl_lock, flags); spin_lock_irqsave(&tty->ctrl_lock, flags);
tty->flags = 0; clear_bit(TTY_THROTTLED, &tty->flags);
clear_bit(TTY_PUSH, &tty->flags);
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
put_pid(tty->session); put_pid(tty->session);
put_pid(tty->pgrp); put_pid(tty->pgrp);
tty->session = NULL; tty->session = NULL;
tty->pgrp = NULL; tty->pgrp = NULL;
tty->ctrl_status = 0; tty->ctrl_status = 0;
set_bit(TTY_HUPPED, &tty->flags);
spin_unlock_irqrestore(&tty->ctrl_lock, flags); spin_unlock_irqrestore(&tty->ctrl_lock, flags);
/* Account for the p->signal references we killed */ /* Account for the p->signal references we killed */
...@@ -660,10 +614,7 @@ static void do_tty_hangup(struct work_struct *work) ...@@ -660,10 +614,7 @@ static void do_tty_hangup(struct work_struct *work)
* can't yet guarantee all that. * can't yet guarantee all that.
*/ */
set_bit(TTY_HUPPED, &tty->flags); set_bit(TTY_HUPPED, &tty->flags);
if (ld) { tty_ldisc_enable(tty);
tty_ldisc_enable(tty);
tty_ldisc_deref(ld);
}
unlock_kernel(); unlock_kernel();
if (f) if (f)
fput(f); fput(f);
...@@ -2570,7 +2521,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ...@@ -2570,7 +2521,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case TIOCGSID: case TIOCGSID:
return tiocgsid(tty, real_tty, p); return tiocgsid(tty, real_tty, p);
case TIOCGETD: case TIOCGETD:
return put_user(tty->ldisc.ops->num, (int __user *)p); return put_user(tty->ldisc->ops->num, (int __user *)p);
case TIOCSETD: case TIOCSETD:
return tiocsetd(tty, p); return tiocsetd(tty, p);
/* /*
...@@ -2785,6 +2736,7 @@ void initialize_tty_struct(struct tty_struct *tty, ...@@ -2785,6 +2736,7 @@ void initialize_tty_struct(struct tty_struct *tty,
tty->buf.head = tty->buf.tail = NULL; tty->buf.head = tty->buf.tail = NULL;
tty_buffer_init(tty); tty_buffer_init(tty);
mutex_init(&tty->termios_mutex); mutex_init(&tty->termios_mutex);
mutex_init(&tty->ldisc_mutex);
init_waitqueue_head(&tty->write_wait); init_waitqueue_head(&tty->write_wait);
init_waitqueue_head(&tty->read_wait); init_waitqueue_head(&tty->read_wait);
INIT_WORK(&tty->hangup_work, do_tty_hangup); INIT_WORK(&tty->hangup_work, do_tty_hangup);
......
This diff is collapsed.
...@@ -226,8 +226,11 @@ struct tty_struct { ...@@ -226,8 +226,11 @@ struct tty_struct {
struct tty_driver *driver; struct tty_driver *driver;
const struct tty_operations *ops; const struct tty_operations *ops;
int index; int index;
/* The ldisc objects are protected by tty_ldisc_lock at the moment */
struct tty_ldisc ldisc; /* Protects ldisc changes: Lock tty not pty */
struct mutex ldisc_mutex;
struct tty_ldisc *ldisc;
struct mutex termios_mutex; struct mutex termios_mutex;
spinlock_t ctrl_lock; spinlock_t ctrl_lock;
/* Termios values are protected by the termios mutex */ /* Termios values are protected by the termios mutex */
...@@ -314,6 +317,7 @@ struct tty_struct { ...@@ -314,6 +317,7 @@ struct tty_struct {
#define TTY_CLOSING 7 /* ->close() in progress */ #define TTY_CLOSING 7 /* ->close() in progress */
#define TTY_LDISC 9 /* Line discipline attached */ #define TTY_LDISC 9 /* Line discipline attached */
#define TTY_LDISC_CHANGING 10 /* Line discipline changing */ #define TTY_LDISC_CHANGING 10 /* Line discipline changing */
#define TTY_LDISC_OPEN 11 /* Line discipline is open */
#define TTY_HW_COOK_OUT 14 /* Hardware can do output cooking */ #define TTY_HW_COOK_OUT 14 /* Hardware can do output cooking */
#define TTY_HW_COOK_IN 15 /* Hardware can do input cooking */ #define TTY_HW_COOK_IN 15 /* Hardware can do input cooking */
#define TTY_PTY_LOCK 16 /* pty private */ #define TTY_PTY_LOCK 16 /* pty private */
...@@ -406,6 +410,7 @@ extern int tty_termios_hw_change(struct ktermios *a, struct ktermios *b); ...@@ -406,6 +410,7 @@ extern int tty_termios_hw_change(struct ktermios *a, struct ktermios *b);
extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *); extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *);
extern void tty_ldisc_deref(struct tty_ldisc *); extern void tty_ldisc_deref(struct tty_ldisc *);
extern struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *); extern struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *);
extern void tty_ldisc_hangup(struct tty_struct *tty);
extern const struct file_operations tty_ldiscs_proc_fops; extern const struct file_operations tty_ldiscs_proc_fops;
extern void tty_wakeup(struct tty_struct *tty); extern void tty_wakeup(struct tty_struct *tty);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment