Created
September 3, 2015 08:46
-
-
Save thejh/c52a7a11266d31d7d659 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From 7f1265b917aba4436653aa8e7bf90976b82b77ee Mon Sep 17 00:00:00 2001 | |
From: Jann Horn <[email protected]> | |
Date: Fri, 14 Aug 2015 17:47:01 +0200 | |
Subject: [PATCH] drivers/tty: require read access for controlling terminal | |
This is mostly a hardening fix, given that write-only access to other | |
users' ttys is usually only given through setgid tty executables. | |
Signed-off-by: Jann Horn <[email protected]> | |
--- | |
drivers/tty/tty_io.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- | |
1 file changed, 44 insertions(+), 4 deletions(-) | |
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c | |
index 57fc6ee..dcd6a83 100644 | |
--- a/drivers/tty/tty_io.c | |
+++ b/drivers/tty/tty_io.c | |
@@ -2136,8 +2136,31 @@ retry_open: | |
if (!noctty && | |
current->signal->leader && | |
!current->signal->tty && | |
- tty->session == NULL) | |
- __proc_set_tty(tty); | |
+ tty->session == NULL) { | |
+ /* | |
+ * Don't let a process that only has write access to the tty | |
+ * obtain the privileges associated with having a tty as | |
+ * controlling terminal (being able to reopen it with full | |
+ * access through /dev/tty, being able to perform pushback). | |
+ * Many distributions set the group of all ttys to "tty" and | |
+ * grant write-only access to all terminals for setgid tty | |
+ * binaries, which should not imply full privileges on all ttys. | |
+ * | |
+ * This could theoretically break old code that performs open() | |
+ * on a write-only file descriptor. In that case, it might be | |
+ * necessary to also permit this if | |
+ * inode_permission(inode, MAY_READ) == 0. | |
+ */ | |
+ if (filp->f_mode & FMODE_READ) | |
+ __proc_set_tty(tty); | |
+ else { | |
+ char comm[sizeof(current->comm)]; | |
+ | |
+ pr_warn_once("%s: silently refused to set controlling terminal on open() - tty is not open for reading. Offending process: %d (%s)\n", | |
+ tty->name, current->pid, | |
+ get_task_comm(comm, current)); | |
+ } | |
+ } | |
spin_unlock_irq(¤t->sighand->siglock); | |
read_unlock(&tasklist_lock); | |
tty_unlock(tty); | |
@@ -2426,7 +2449,7 @@ static int fionbio(struct file *file, int __user *p) | |
* Takes ->siglock() when updating signal->tty | |
*/ | |
-static int tiocsctty(struct tty_struct *tty, int arg) | |
+static int tiocsctty(struct tty_struct *tty, struct file *file, int arg) | |
{ | |
int ret = 0; | |
@@ -2460,6 +2483,23 @@ static int tiocsctty(struct tty_struct *tty, int arg) | |
goto unlock; | |
} | |
} | |
+ | |
+ /* See the comment in tty_open(). */ | |
+ if ((file->f_mode & FMODE_READ) == 0) { | |
+ char comm[sizeof(current->comm)]; | |
+ | |
+ get_task_comm(comm, current); | |
+ if (capable(CAP_SYS_ADMIN)) { | |
+ pr_warn_once("%s: TIOCSCTTY on a write-only fd was only allowed because it was performed by root. Offending process: %d (%s)\n", | |
+ tty->name, current->pid, comm); | |
+ } else { | |
+ pr_warn_once("%s: TIOCSCTTY on a write-only fd was refused for security reasons! This might break old code. Offending process: %d (%s)\n", | |
+ tty->name, current->pid, comm); | |
+ ret = -EPERM; | |
+ goto unlock; | |
+ } | |
+ } | |
+ | |
proc_set_tty(tty); | |
unlock: | |
read_unlock(&tasklist_lock); | |
@@ -2852,7 +2892,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg) | |
no_tty(); | |
return 0; | |
case TIOCSCTTY: | |
- return tiocsctty(tty, arg); | |
+ return tiocsctty(tty, file, arg); | |
case TIOCGPGRP: | |
return tiocgpgrp(tty, real_tty, p); | |
case TIOCSPGRP: | |
-- | |
2.1.4 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment