All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>
Cc: linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	Eric Dumazet <edumazet@google.com>,
	Nicolas Pitre <nico@fluxnic.net>,
	Alan Cox <gnomes@lxorguk.ukuu.org.uk>
Subject: [PATCH] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console
Date: Sun, 23 Feb 2020 23:12:47 -0800	[thread overview]
Message-ID: <20200224071247.283098-1-ebiggers@kernel.org> (raw)
In-Reply-To: <0000000000006663de0598d25ab1@google.com>

From: Eric Biggers <ebiggers@google.com>

The VT_DISALLOCATE ioctl can free a virtual console while tty_release()
is still running, causing a use-after-free in con_shutdown().  This
occurs because VT_DISALLOCATE only considers a virtual console to be
in-use if it has a tty_struct with count > 0.  But actually when
count == 0, the tty is still in the process of being closed.

Fix this by treating a virtual console as in-use if it has a tty_struct
at all, even with zero count; and by making VT_DISALLOCATE take the
tty_mutex in order to provide synchronization with release_tty().

Reproducer:
	#include <fcntl.h>
	#include <linux/vt.h>
	#include <sys/ioctl.h>
	#include <unistd.h>

	int main()
	{
		if (fork()) {
			for (;;)
				close(open("/dev/tty5", O_RDWR));
		} else {
			int fd = open("/dev/tty10", O_RDWR);

			for (;;)
				ioctl(fd, VT_DISALLOCATE, 5);
		}
	}

KASAN report:
	BUG: KASAN: use-after-free in con_shutdown+0x76/0x80 drivers/tty/vt/vt.c:3278
	Write of size 8 at addr ffff88806a4ec108 by task syz_vt/129

	CPU: 0 PID: 129 Comm: syz_vt Not tainted 5.6.0-rc2 #11
	Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20191223_100556-anatol 04/01/2014
	Call Trace:
	 [...]
	 con_shutdown+0x76/0x80 drivers/tty/vt/vt.c:3278
	 release_tty+0xa8/0x410 drivers/tty/tty_io.c:1514
	 tty_release_struct+0x34/0x50 drivers/tty/tty_io.c:1629
	 tty_release+0x984/0xed0 drivers/tty/tty_io.c:1789
	 [...]

	Allocated by task 129:
	 [...]
	 kzalloc include/linux/slab.h:669 [inline]
	 vc_allocate drivers/tty/vt/vt.c:1085 [inline]
	 vc_allocate+0x1ac/0x680 drivers/tty/vt/vt.c:1066
	 con_install+0x4d/0x3f0 drivers/tty/vt/vt.c:3229
	 tty_driver_install_tty drivers/tty/tty_io.c:1228 [inline]
	 tty_init_dev+0x94/0x350 drivers/tty/tty_io.c:1341
	 tty_open_by_driver drivers/tty/tty_io.c:1987 [inline]
	 tty_open+0x3ca/0xb30 drivers/tty/tty_io.c:2035
	 [...]

	Freed by task 130:
	 [...]
	 kfree+0xbf/0x1e0 mm/slab.c:3757
	 vt_disallocate drivers/tty/vt/vt_ioctl.c:300 [inline]
	 vt_ioctl+0x16dc/0x1e30 drivers/tty/vt/vt_ioctl.c:818
	 tty_ioctl+0x9db/0x11b0 drivers/tty/tty_io.c:2660
	 [...]

Fixes: 4001d7b7fc27 ("vt: push down the tty lock so we can see what is left to tackle")
Cc: <stable@vger.kernel.org> # v3.4+
Reported-by: syzbot+522643ab5729b0421998@syzkaller.appspotmail.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/tty/vt/vt_ioctl.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index ee6c91ef1f6cf..57d681706fa85 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -42,7 +42,7 @@
 char vt_dont_switch;
 extern struct tty_driver *console_driver;
 
-#define VT_IS_IN_USE(i)	(console_driver->ttys[i] && console_driver->ttys[i]->count)
+#define VT_IS_IN_USE(i)	(console_driver->ttys[i] != NULL)
 #define VT_BUSY(i)	(VT_IS_IN_USE(i) || i == fg_console || vc_cons[i].d == sel_cons)
 
 /*
@@ -288,12 +288,14 @@ static int vt_disallocate(unsigned int vc_num)
 	struct vc_data *vc = NULL;
 	int ret = 0;
 
+	mutex_lock(&tty_mutex); /* synchronize with release_tty() */
 	console_lock();
 	if (VT_BUSY(vc_num))
 		ret = -EBUSY;
 	else if (vc_num)
 		vc = vc_deallocate(vc_num);
 	console_unlock();
+	mutex_unlock(&tty_mutex);
 
 	if (vc && vc_num >= MIN_NR_CONSOLES) {
 		tty_port_destroy(&vc->port);
@@ -309,6 +311,7 @@ static void vt_disallocate_all(void)
 	struct vc_data *vc[MAX_NR_CONSOLES];
 	int i;
 
+	mutex_lock(&tty_mutex); /* synchronize with release_tty() */
 	console_lock();
 	for (i = 1; i < MAX_NR_CONSOLES; i++)
 		if (!VT_BUSY(i))
@@ -316,6 +319,7 @@ static void vt_disallocate_all(void)
 		else
 			vc[i] = NULL;
 	console_unlock();
+	mutex_unlock(&tty_mutex);
 
 	for (i = 1; i < MAX_NR_CONSOLES; i++) {
 		if (vc[i] && i >= MIN_NR_CONSOLES) {
-- 
2.25.1


  reply	other threads:[~2020-02-24  7:15 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 20:15 KASAN: use-after-free Write in release_tty syzbot
2020-02-24  7:12 ` Eric Biggers [this message]
2020-02-24  8:04   ` [PATCH] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Jiri Slaby
2020-02-24  8:19     ` Eric Biggers
2020-03-02 21:23       ` Eric Biggers
2020-03-18 10:06         ` Greg Kroah-Hartman
2020-03-18 10:10           ` Jiri Slaby
2020-03-18 13:15       ` Jiri Slaby
2020-03-18 22:27         ` Eric Biggers
2020-03-18 22:38           ` [PATCH v2 0/2] vt: fix some vt_ioctl races Eric Biggers
2020-03-18 22:38             ` [PATCH v2 1/2] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Eric Biggers
2020-03-19  7:36               ` Jiri Slaby
2020-03-20  5:10                 ` Eric Biggers
2020-03-20  6:57                   ` Greg Kroah-Hartman
2020-03-18 22:38             ` [PATCH v2 2/2] vt: vt_ioctl: fix use-after-free in vt_in_use() Eric Biggers
2020-03-20 13:42               ` Jiri Slaby
2020-03-20 19:34                 ` Eric Biggers
2020-03-22  3:43                   ` [PATCH v3 0/2] vt: fix some vt_ioctl races Eric Biggers
2020-03-22  3:43                     ` [PATCH v3 1/2] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Eric Biggers
2020-03-27 10:28                       ` Jiri Slaby
2020-03-22  3:43                     ` [PATCH v3 2/2] vt: vt_ioctl: fix use-after-free in vt_in_use() Eric Biggers
2020-03-27 10:30                       ` Jiri Slaby
2020-03-24 11:29                     ` [PATCH v3 0/2] vt: fix some vt_ioctl races Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200224071247.283098-1-ebiggers@kernel.org \
    --to=ebiggers@kernel.org \
    --cc=edumazet@google.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nico@fluxnic.net \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.