redonkable
/
alistair23-linux
1
0
Fork 0
Code Releases Activity

redo locking of tty->pgrp 

Historically tty->pgrp and friends were pid_t and the code "knew" they were
safe.  The change to pid structs opened up a few races and the removal of the
BKL in places made them quite hittable.  We put tty->pgrp under the ctrl_lock
for the tty.

Signed-off-by: Alan Cox <alan@redhat.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
hifive-unleashed-5.1
Alan Cox 2008-04-30 00:53:30 -07:00 committed by Linus Torvalds
parent 04f378b198
commit 47f86834bb
3 changed files with 72 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
drivers/char/tty_io.c
View File

@ -1204,26 +1204,37 @@ EXPORT_SYMBOL_GPL(tty_find_polling_driver);
* not in the foreground, send a SIGTTOU. If the signal is blocked or * not in the foreground, send a SIGTTOU. If the signal is blocked or
* ignored, go ahead and perform the operation. (POSIX 7.2) * ignored, go ahead and perform the operation. (POSIX 7.2)
* *
* Locking: none - FIXME: review this * Locking: ctrl_lock - FIXME: review this
*/ */
int tty_check_change(struct tty_struct *tty) int tty_check_change(struct tty_struct *tty)
{ {
unsigned long flags;
int ret = 0;
if (current->signal->tty != tty) if (current->signal->tty != tty)
return 0; return 0;
spin_lock_irqsave(&tty->ctrl_lock, flags);
if (!tty->pgrp) { if (!tty->pgrp) {
printk(KERN_WARNING "tty_check_change: tty->pgrp == NULL!\n"); printk(KERN_WARNING "tty_check_change: tty->pgrp == NULL!\n");
return 0; goto out;
} }
if (task_pgrp(current) == tty->pgrp) if (task_pgrp(current) == tty->pgrp)
return 0; goto out;
if (is_ignored(SIGTTOU)) if (is_ignored(SIGTTOU))
return 0; goto out;
if (is_current_pgrp_orphaned()) if (is_current_pgrp_orphaned()) {
return -EIO; ret = -EIO;
goto out;
}
kill_pgrp(task_pgrp(current), SIGTTOU, 1); kill_pgrp(task_pgrp(current), SIGTTOU, 1);
set_thread_flag(TIF_SIGPENDING); set_thread_flag(TIF_SIGPENDING);
return -ERESTARTSYS; ret = -ERESTARTSYS;
out:
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
return ret;
} }
EXPORT_SYMBOL(tty_check_change); EXPORT_SYMBOL(tty_check_change);
@ -1403,6 +1414,7 @@ static void do_tty_hangup(struct work_struct *work)
struct task_struct *p; struct task_struct *p;
struct tty_ldisc *ld; struct tty_ldisc *ld;
int closecount = 0, n; int closecount = 0, n;
unsigned long flags;
if (!tty) if (!tty)
return; return;
@ -1479,19 +1491,24 @@ static void do_tty_hangup(struct work_struct *work)
__group_send_sig_info(SIGHUP, SEND_SIG_PRIV, p); __group_send_sig_info(SIGHUP, SEND_SIG_PRIV, p);
__group_send_sig_info(SIGCONT, SEND_SIG_PRIV, p); __group_send_sig_info(SIGCONT, SEND_SIG_PRIV, p);
put_pid(p->signal->tty_old_pgrp); /* A noop */ put_pid(p->signal->tty_old_pgrp); /* A noop */
spin_lock_irqsave(&tty->ctrl_lock, flags);
if (tty->pgrp) if (tty->pgrp)
p->signal->tty_old_pgrp = get_pid(tty->pgrp); p->signal->tty_old_pgrp = get_pid(tty->pgrp);
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
spin_unlock_irq(&p->sighand->siglock); spin_unlock_irq(&p->sighand->siglock);
} while_each_pid_task(tty->session, PIDTYPE_SID, p); } while_each_pid_task(tty->session, PIDTYPE_SID, p);
} }
read_unlock(&tasklist_lock); read_unlock(&tasklist_lock);
spin_lock_irqsave(&tty->ctrl_lock, flags);
tty->flags = 0; tty->flags = 0;
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;
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
/* /*
* If one of the devices matches a console pointer, we * If one of the devices matches a console pointer, we
* cannot just call hangup() because that will cause * cannot just call hangup() because that will cause
@ -1666,10 +1683,13 @@ void disassociate_ctty(int on_exit)
/* It is possible that do_tty_hangup has free'd this tty */ /* It is possible that do_tty_hangup has free'd this tty */
tty = get_current_tty(); tty = get_current_tty();
if (tty) { if (tty) {
unsigned long flags;
spin_lock_irqsave(&tty->ctrl_lock, 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;
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
} else { } else {
#ifdef TTY_DEBUG_HANGUP #ifdef TTY_DEBUG_HANGUP
printk(KERN_DEBUG "error attempted to write to tty [0x%p]" printk(KERN_DEBUG "error attempted to write to tty [0x%p]"
@ -1785,10 +1805,8 @@ EXPORT_SYMBOL(start_tty);
* for hung up devices before calling the line discipline method. * for hung up devices before calling the line discipline method.
* *
* Locking: * Locking:
* Locks the line discipline internally while needed * Locks the line discipline internally while needed. Multiple
* For historical reasons the line discipline read method is * read calls may be outstanding in parallel.
* invoked under the BKL. This will go away in time so do not rely on it
* in new code. Multiple read calls may be outstanding in parallel.
*/ */
static ssize_t tty_read(struct file *file, char __user *buf, size_t count, static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
@ -2888,6 +2906,7 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
static int tty_fasync(int fd, struct file *filp, int on) static int tty_fasync(int fd, struct file *filp, int on)
{ {
struct tty_struct *tty; struct tty_struct *tty;
unsigned long flags;
int retval; int retval;
tty = (struct tty_struct *)filp->private_data; tty = (struct tty_struct *)filp->private_data;
@ -2903,6 +2922,7 @@ static int tty_fasync(int fd, struct file *filp, int on)
struct pid *pid; struct pid *pid;
if (!waitqueue_active(&tty->read_wait)) if (!waitqueue_active(&tty->read_wait))
tty->minimum_to_wake = 1; tty->minimum_to_wake = 1;
spin_lock_irqsave(&tty->ctrl_lock, flags);
if (tty->pgrp) { if (tty->pgrp) {
pid = tty->pgrp; pid = tty->pgrp;
type = PIDTYPE_PGID; type = PIDTYPE_PGID;
@ -2910,6 +2930,7 @@ static int tty_fasync(int fd, struct file *filp, int on)
pid = task_pid(current); pid = task_pid(current);
type = PIDTYPE_PID; type = PIDTYPE_PID;
} }
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
retval = __f_setown(filp, pid, type, 0); retval = __f_setown(filp, pid, type, 0);
if (retval) if (retval)
return retval; return retval;
@ -2995,6 +3016,8 @@ static int tiocswinsz(struct tty_struct *tty, struct tty_struct *real_tty,
struct winsize __user *arg) struct winsize __user *arg)
{ {
struct winsize tmp_ws; struct winsize tmp_ws;
struct pid *pgrp, *rpgrp;
unsigned long flags;
if (copy_from_user(&tmp_ws, arg, sizeof(*arg))) if (copy_from_user(&tmp_ws, arg, sizeof(*arg)))
return -EFAULT; return -EFAULT;
@ -3012,10 +3035,21 @@ static int tiocswinsz(struct tty_struct *tty, struct tty_struct *real_tty,
} }
} }
#endif #endif
if (tty->pgrp) /* Get the PID values and reference them so we can
kill_pgrp(tty->pgrp, SIGWINCH, 1); avoid holding the tty ctrl lock while sending signals */
if ((real_tty->pgrp != tty->pgrp) && real_tty->pgrp) spin_lock_irqsave(&tty->ctrl_lock, flags);
kill_pgrp(real_tty->pgrp, SIGWINCH, 1); pgrp = get_pid(tty->pgrp);
rpgrp = get_pid(real_tty->pgrp);
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
if (pgrp)
kill_pgrp(pgrp, SIGWINCH, 1);
if (rpgrp != pgrp && rpgrp)
kill_pgrp(rpgrp, SIGWINCH, 1);
put_pid(pgrp);
put_pid(rpgrp);
tty->winsize = tmp_ws; tty->winsize = tmp_ws;
real_tty->winsize = tmp_ws; real_tty->winsize = tmp_ws;
done: done:
@ -3171,7 +3205,7 @@ static int tiocgpgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
* Set the process group of the tty to the session passed. Only * Set the process group of the tty to the session passed. Only
* permitted where the tty session is our session. * permitted where the tty session is our session.
* *
* Locking: RCU * Locking: RCU, ctrl lock
*/ */
static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t __user *p) static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t __user *p)
@ -3179,6 +3213,7 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
struct pid *pgrp; struct pid *pgrp;
pid_t pgrp_nr; pid_t pgrp_nr;
int retval = tty_check_change(real_tty); int retval = tty_check_change(real_tty);
unsigned long flags;
if (retval == -EIO) if (retval == -EIO)
return -ENOTTY; return -ENOTTY;
@ -3201,8 +3236,10 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
if (session_of_pgrp(pgrp) != task_session(current)) if (session_of_pgrp(pgrp) != task_session(current))
goto out_unlock; goto out_unlock;
retval = 0; retval = 0;
spin_lock_irqsave(&tty->ctrl_lock, flags);
put_pid(real_tty->pgrp); put_pid(real_tty->pgrp);
real_tty->pgrp = get_pid(pgrp); real_tty->pgrp = get_pid(pgrp);
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
out_unlock: out_unlock:
rcu_read_unlock(); rcu_read_unlock();
return retval; return retval;
@ -4077,14 +4114,19 @@ void proc_clear_tty(struct task_struct *p)
} }
EXPORT_SYMBOL(proc_clear_tty); EXPORT_SYMBOL(proc_clear_tty);
/* Called under the sighand lock */
static void __proc_set_tty(struct task_struct *tsk, struct tty_struct *tty) static void __proc_set_tty(struct task_struct *tsk, struct tty_struct *tty)
{ {
if (tty) { if (tty) {
/* We should not have a session or pgrp to here but.... */ unsigned long flags;
/* We should not have a session or pgrp to put here but.... */
spin_lock_irqsave(&tty->ctrl_lock, flags);
put_pid(tty->session); put_pid(tty->session);
put_pid(tty->pgrp); put_pid(tty->pgrp);
tty->session = get_pid(task_session(tsk));
tty->pgrp = get_pid(task_pgrp(tsk)); tty->pgrp = get_pid(task_pgrp(tsk));
spin_unlock_irqrestore(&tty->ctrl_lock, flags);
tty->session = get_pid(task_session(tsk));
} }
put_pid(tsk->signal->tty_old_pgrp); put_pid(tsk->signal->tty_old_pgrp);
tsk->signal->tty = tty; tsk->signal->tty = tty;

6
drivers/char/vt.c
View File

@ -909,15 +909,21 @@ int vc_resize(struct vc_data *vc, unsigned int cols, unsigned int lines)
if (vc->vc_tty) { if (vc->vc_tty) {
struct winsize ws, *cws = &vc->vc_tty->winsize; struct winsize ws, *cws = &vc->vc_tty->winsize;
unsigned long flags;
memset(&ws, 0, sizeof(ws)); memset(&ws, 0, sizeof(ws));
ws.ws_row = vc->vc_rows; ws.ws_row = vc->vc_rows;
ws.ws_col = vc->vc_cols; ws.ws_col = vc->vc_cols;
ws.ws_ypixel = vc->vc_scan_lines; ws.ws_ypixel = vc->vc_scan_lines;
mutex_lock(&vc->vc_tty->termios_mutex);
spin_lock_irqsave(&vc->vc_tty->ctrl_lock, flags);
if ((ws.ws_row != cws->ws_row || ws.ws_col != cws->ws_col) && if ((ws.ws_row != cws->ws_row || ws.ws_col != cws->ws_col) &&
vc->vc_tty->pgrp) vc->vc_tty->pgrp)
kill_pgrp(vc->vc_tty->pgrp, SIGWINCH, 1); kill_pgrp(vc->vc_tty->pgrp, SIGWINCH, 1);
spin_unlock_irqrestore(&vc->vc_tty->ctrl_lock, flags);
*cws = ws; *cws = ws;
mutex_unlock(&vc->vc_tty->termios_mutex);
} }
if (CON_IS_VISIBLE(vc)) if (CON_IS_VISIBLE(vc))

10
include/linux/tty.h
View File

@ -184,21 +184,22 @@ struct tty_struct {
struct tty_ldisc ldisc; 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 */
struct ktermios *termios, *termios_locked; struct ktermios *termios, *termios_locked;
char name[64]; char name[64];
struct pid *pgrp; struct pid *pgrp; /* Protected by ctrl lock */
struct pid *session; struct pid *session;
unsigned long flags; unsigned long flags;
int count; int count;
struct winsize winsize; struct winsize winsize; /* termios mutex */
unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1; unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
unsigned char low_latency:1, warned:1; unsigned char low_latency:1, warned:1;
unsigned char ctrl_status; unsigned char ctrl_status; /* ctrl_lock */
unsigned int receive_room; /* Bytes free for queue */ unsigned int receive_room; /* Bytes free for queue */
struct tty_struct *link; struct tty_struct *link;
struct fasync_struct *fasync; struct fasync_struct *fasync;
struct tty_bufhead buf; struct tty_bufhead buf; /* Locked internally */
int alt_speed; /* For magic substitution of 38400 bps */ int alt_speed; /* For magic substitution of 38400 bps */
wait_queue_head_t write_wait; wait_queue_head_t write_wait;
wait_queue_head_t read_wait; wait_queue_head_t read_wait;
@ -212,6 +213,7 @@ struct tty_struct {
/* /*
* The following is data for the N_TTY line discipline. For * The following is data for the N_TTY line discipline. For
* historical reasons, this is included in the tty structure. * historical reasons, this is included in the tty structure.
* Mostly locked by the BKL.
*/ */
unsigned int column; unsigned int column;
unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1; unsigned char lnext:1, erasing:1, raw:1, real_raw:1, icanon:1;