All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	syzbot+522643ab5729b0421998@syzkaller.appspotmail.com,
	Jiri Slaby <jslaby@suse.cz>, Eric Biggers <ebiggers@google.com>
Subject: [PATCH 5.4 08/27] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console
Date: Wed,  1 Apr 2020 18:17:36 +0200	[thread overview]
Message-ID: <20200401161421.844438426@linuxfoundation.org> (raw)
In-Reply-To: <20200401161414.352722470@linuxfoundation.org>

From: Eric Biggers <ebiggers@google.com>

commit ca4463bf8438b403596edd0ec961ca0d4fbe0220 upstream.

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 considers a virtual console's
'struct vc_data' to be unused as soon as the corresponding tty's
refcount hits 0.  But actually it may be still being closed.

Fix this by making vc_data be reference-counted via the embedded
'struct tty_port'.  A newly allocated virtual console has refcount 1.
Opening it for the first time increments the refcount to 2.  Closing it
for the last time decrements the refcount (in tty_operations::cleanup()
so that it happens late enough), as does VT_DISALLOCATE.

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
Acked-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20200322034305.210082-2-ebiggers@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/tty/vt/vt.c       |   23 ++++++++++++++++++++++-
 drivers/tty/vt/vt_ioctl.c |   12 ++++--------
 2 files changed, 26 insertions(+), 9 deletions(-)

--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1075,6 +1075,17 @@ static void visual_deinit(struct vc_data
 	module_put(vc->vc_sw->owner);
 }
 
+static void vc_port_destruct(struct tty_port *port)
+{
+	struct vc_data *vc = container_of(port, struct vc_data, port);
+
+	kfree(vc);
+}
+
+static const struct tty_port_operations vc_port_ops = {
+	.destruct = vc_port_destruct,
+};
+
 int vc_allocate(unsigned int currcons)	/* return 0 on success */
 {
 	struct vt_notifier_param param;
@@ -1100,6 +1111,7 @@ int vc_allocate(unsigned int currcons)	/
 
 	vc_cons[currcons].d = vc;
 	tty_port_init(&vc->port);
+	vc->port.ops = &vc_port_ops;
 	INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
 
 	visual_init(vc, currcons, 1);
@@ -3254,6 +3266,7 @@ static int con_install(struct tty_driver
 
 	tty->driver_data = vc;
 	vc->port.tty = tty;
+	tty_port_get(&vc->port);
 
 	if (!tty->winsize.ws_row && !tty->winsize.ws_col) {
 		tty->winsize.ws_row = vc_cons[currcons].d->vc_rows;
@@ -3289,6 +3302,13 @@ static void con_shutdown(struct tty_stru
 	console_unlock();
 }
 
+static void con_cleanup(struct tty_struct *tty)
+{
+	struct vc_data *vc = tty->driver_data;
+
+	tty_port_put(&vc->port);
+}
+
 static int default_color           = 7; /* white */
 static int default_italic_color    = 2; // green (ASCII)
 static int default_underline_color = 3; // cyan (ASCII)
@@ -3413,7 +3433,8 @@ static const struct tty_operations con_o
 	.throttle = con_throttle,
 	.unthrottle = con_unthrottle,
 	.resize = vt_resize,
-	.shutdown = con_shutdown
+	.shutdown = con_shutdown,
+	.cleanup = con_cleanup,
 };
 
 static struct cdev vc0_cdev;
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -310,10 +310,8 @@ static int vt_disallocate(unsigned int v
 		vc = vc_deallocate(vc_num);
 	console_unlock();
 
-	if (vc && vc_num >= MIN_NR_CONSOLES) {
-		tty_port_destroy(&vc->port);
-		kfree(vc);
-	}
+	if (vc && vc_num >= MIN_NR_CONSOLES)
+		tty_port_put(&vc->port);
 
 	return ret;
 }
@@ -333,10 +331,8 @@ static void vt_disallocate_all(void)
 	console_unlock();
 
 	for (i = 1; i < MAX_NR_CONSOLES; i++) {
-		if (vc[i] && i >= MIN_NR_CONSOLES) {
-			tty_port_destroy(&vc[i]->port);
-			kfree(vc[i]);
-		}
+		if (vc[i] && i >= MIN_NR_CONSOLES)
+			tty_port_put(&vc[i]->port);
 	}
 }
 



  parent reply	other threads:[~2020-04-01 16:22 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 16:17 [PATCH 5.4 00/27] 5.4.30-rc1 review Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 01/27] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 02/27] mac80211: fix authentication with iwlwifi/mvm Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 03/27] serial: sprd: Fix a dereference warning Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 04/27] vt: selection, introduce vc_is_sel Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 05/27] vt: ioctl, switch VT_IS_IN_USE and VT_BUSY to inlines Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 06/27] vt: switch vt_dont_switch to bool Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 07/27] vt: vt_ioctl: remove unnecessary console allocation checks Greg Kroah-Hartman
2020-04-01 16:17 ` Greg Kroah-Hartman [this message]
2020-04-01 16:17 ` [PATCH 5.4 09/27] vt: vt_ioctl: fix use-after-free in vt_in_use() Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 10/27] platform/x86: pmc_atom: Add Lex 2I385SW to critclk_systems DMI table Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 11/27] bpf: Explicitly memset the bpf_attr structure Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 12/27] bpf: Explicitly memset some bpf info structures declared on the stack Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 13/27] gpiolib: acpi: Add quirk to ignore EC wakeups on HP x2 10 CHT + AXP288 model Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 14/27] net: ks8851-ml: Fix IO operations, again Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 15/27] clk: imx: Align imx sc clock msg structs to 4 Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 16/27] clk: imx: Align imx sc clock parent " Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 17/27] clk: ti: am43xx: Fix clock parent for RTC clock Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 18/27] libceph: fix alloc_msg_with_page_vector() memory leaks Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 19/27] arm64: alternative: fix build with clang integrated assembler Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 20/27] perf map: Fix off by one in strncpy() size argument Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 21/27] ARM: dts: oxnas: Fix clear-mask property Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 22/27] ARM: bcm2835-rpi-zero-w: Add missing pinctrl name Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 23/27] ARM: dts: imx6: phycore-som: fix arm and soc minimum voltage Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 24/27] ARM: dts: N900: fix onenand timings Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 25/27] ARM: dts: sun8i: r40: Move AHCI device node based on address order Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 26/27] arm64: dts: ls1043a-rdb: correct RGMII delay mode to rgmii-id Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.4 27/27] arm64: dts: ls1046ardb: set RGMII interfaces to RGMII_ID mode Greg Kroah-Hartman
2020-04-02  0:13 ` [PATCH 5.4 00/27] 5.4.30-rc1 review Guenter Roeck
2020-04-02  7:10 ` Jon Hunter
2020-04-02  7:10   ` Jon Hunter
2020-04-02  9:11 ` Naresh Kamboju
2020-04-02 16:51 ` shuah

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=20200401161421.844438426@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ebiggers@google.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+522643ab5729b0421998@syzkaller.appspotmail.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.