Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc8bcd9c

Browse files
thejhgregkh
authored andcommitted
tty: Fix ->session locking
Currently, locking of ->session is very inconsistent; most placesprotect it using the legacy tty mutex, but disassociate_ctty(),__do_SAK(), tiocspgrp() and tiocgsid() don't.Two of the writers hold the ctrl_lock (because they already need it for->pgrp), but __proc_set_tty() doesn't do that yet.On a PREEMPT=y system, an unprivileged user can theoretically abusethis broken locking to read 4 bytes of freed memory via TIOCGSID iftiocgsid() is preempted long enough at the right point. (Other thingsmight also go wrong, especially if root-only ioctls are involved; I'mnot sure about that.)Change the locking on ->session such that: - tty_lock() is held by all writers: By making disassociate_ctty() hold it. This should be fine because the same lock can already be taken through the call to tty_vhangup_session(). The tricky part is that we need to shorten the area covered by siglock to be able to take tty_lock() without ugly retry logic; as far as I can tell, this should be fine, since nothing in the signal_struct is touched in the `if (tty)` branch. - ctrl_lock is held by all writers: By changing __proc_set_tty() to hold the lock a little longer. - All readers that aren't holding tty_lock() hold ctrl_lock: By adding locking to tiocgsid() and __do_SAK(), and expanding the area covered by ctrl_lock in tiocspgrp().Cc: stable@kernel.orgSigned-off-by: Jann Horn <jannh@google.com>Reviewed-by: Jiri Slaby <jirislaby@kernel.org>Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent54ffccb commitc8bcd9c

File tree

3 files changed

+41
-14
lines changed

3 files changed

+41
-14
lines changed

‎drivers/tty/tty_io.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2897,10 +2897,14 @@ void __do_SAK(struct tty_struct *tty)
28972897
structtask_struct*g,*p;
28982898
structpid*session;
28992899
inti;
2900+
unsigned longflags;
29002901

29012902
if (!tty)
29022903
return;
2903-
session=tty->session;
2904+
2905+
spin_lock_irqsave(&tty->ctrl_lock,flags);
2906+
session=get_pid(tty->session);
2907+
spin_unlock_irqrestore(&tty->ctrl_lock,flags);
29042908

29052909
tty_ldisc_flush(tty);
29062910

@@ -2932,6 +2936,7 @@ void __do_SAK(struct tty_struct *tty)
29322936
task_unlock(p);
29332937
}while_each_thread(g,p);
29342938
read_unlock(&tasklist_lock);
2939+
put_pid(session);
29352940
#endif
29362941
}
29372942

‎drivers/tty/tty_jobctrl.c‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ static void __proc_set_tty(struct tty_struct *tty)
103103
put_pid(tty->session);
104104
put_pid(tty->pgrp);
105105
tty->pgrp=get_pid(task_pgrp(current));
106-
spin_unlock_irqrestore(&tty->ctrl_lock,flags);
107106
tty->session=get_pid(task_session(current));
107+
spin_unlock_irqrestore(&tty->ctrl_lock,flags);
108108
if (current->signal->tty) {
109109
tty_debug(tty,"current tty %s not NULL!!\n",
110110
current->signal->tty->name);
@@ -293,20 +293,23 @@ void disassociate_ctty(int on_exit)
293293
spin_lock_irq(&current->sighand->siglock);
294294
put_pid(current->signal->tty_old_pgrp);
295295
current->signal->tty_old_pgrp=NULL;
296-
297296
tty=tty_kref_get(current->signal->tty);
297+
spin_unlock_irq(&current->sighand->siglock);
298+
298299
if (tty) {
299300
unsigned longflags;
301+
302+
tty_lock(tty);
300303
spin_lock_irqsave(&tty->ctrl_lock,flags);
301304
put_pid(tty->session);
302305
put_pid(tty->pgrp);
303306
tty->session=NULL;
304307
tty->pgrp=NULL;
305308
spin_unlock_irqrestore(&tty->ctrl_lock,flags);
309+
tty_unlock(tty);
306310
tty_kref_put(tty);
307311
}
308312

309-
spin_unlock_irq(&current->sighand->siglock);
310313
/* Now clear signal->tty under the lock */
311314
read_lock(&tasklist_lock);
312315
session_clear_tty(task_session(current));
@@ -477,14 +480,19 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
477480
return-ENOTTY;
478481
if (retval)
479482
returnretval;
480-
if (!current->signal->tty||
481-
(current->signal->tty!=real_tty)||
482-
(real_tty->session!=task_session(current)))
483-
return-ENOTTY;
483+
484484
if (get_user(pgrp_nr,p))
485485
return-EFAULT;
486486
if (pgrp_nr<0)
487487
return-EINVAL;
488+
489+
spin_lock_irq(&real_tty->ctrl_lock);
490+
if (!current->signal->tty||
491+
(current->signal->tty!=real_tty)||
492+
(real_tty->session!=task_session(current))) {
493+
retval=-ENOTTY;
494+
gotoout_unlock_ctrl;
495+
}
488496
rcu_read_lock();
489497
pgrp=find_vpid(pgrp_nr);
490498
retval=-ESRCH;
@@ -494,12 +502,12 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
494502
if (session_of_pgrp(pgrp)!=task_session(current))
495503
gotoout_unlock;
496504
retval=0;
497-
spin_lock_irq(&real_tty->ctrl_lock);
498505
put_pid(real_tty->pgrp);
499506
real_tty->pgrp=get_pid(pgrp);
500-
spin_unlock_irq(&real_tty->ctrl_lock);
501507
out_unlock:
502508
rcu_read_unlock();
509+
out_unlock_ctrl:
510+
spin_unlock_irq(&real_tty->ctrl_lock);
503511
returnretval;
504512
}
505513

@@ -511,20 +519,30 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t
511519
*
512520
*Obtain the session id of the tty. If there is no session
513521
*return an error.
514-
*
515-
*Locking: none. Reference to current->signal->tty is safe.
516522
*/
517523
staticinttiocgsid(structtty_struct*tty,structtty_struct*real_tty,pid_t__user*p)
518524
{
525+
unsigned longflags;
526+
pid_tsid;
527+
519528
/*
520529
* (tty == real_tty) is a cheap way of
521530
* testing if the tty is NOT a master pty.
522531
*/
523532
if (tty==real_tty&&current->signal->tty!=real_tty)
524533
return-ENOTTY;
534+
535+
spin_lock_irqsave(&real_tty->ctrl_lock,flags);
525536
if (!real_tty->session)
526-
return-ENOTTY;
527-
returnput_user(pid_vnr(real_tty->session),p);
537+
gotoerr;
538+
sid=pid_vnr(real_tty->session);
539+
spin_unlock_irqrestore(&real_tty->ctrl_lock,flags);
540+
541+
returnput_user(sid,p);
542+
543+
err:
544+
spin_unlock_irqrestore(&real_tty->ctrl_lock,flags);
545+
return-ENOTTY;
528546
}
529547

530548
/*

‎include/linux/tty.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,10 @@ struct tty_struct {
306306
structtermiox*termiox;/* May be NULL for unsupported */
307307
charname[64];
308308
structpid*pgrp;/* Protected by ctrl lock */
309+
/*
310+
* Writes protected by both ctrl lock and legacy mutex, readers must use
311+
* at least one of them.
312+
*/
309313
structpid*session;
310314
unsigned longflags;
311315
intcount;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp