stable.vger.kernel.org archive mirror
 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, Jiri Slaby <jslaby@suse.cz>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH 5.5 10/30] vt: vt_ioctl: fix use-after-free in vt_in_use()
Date: Wed,  1 Apr 2020 18:17:14 +0200	[thread overview]
Message-ID: <20200401161423.206522162@linuxfoundation.org> (raw)
In-Reply-To: <20200401161414.345528747@linuxfoundation.org>

From: Eric Biggers <ebiggers@google.com>

commit 7cf64b18b0b96e751178b8d0505d8466ff5a448f upstream.

vt_in_use() dereferences console_driver->ttys[i] without proper locking.
This is broken because the tty can be closed and freed concurrently.

We could fix this by using 'READ_ONCE(console_driver->ttys[i]) != NULL'
and skipping the check of tty_struct::count.  But, looking at
console_driver->ttys[i] isn't really appropriate anyway because even if
it is NULL the tty can still be in the process of being closed.

Instead, fix it by making vt_in_use() require console_lock() and check
whether the vt is allocated and has port refcount > 1.  This works since
following the patch "vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use
virtual console" the port refcount is incremented while the vt is open.

Reproducer (very unreliable, but it worked for me after a few minutes):

	#include <fcntl.h>
	#include <linux/vt.h>

	int main()
	{
		int fd, nproc;
		struct vt_stat state;
		char ttyname[16];

		fd = open("/dev/tty10", O_RDONLY);
		for (nproc = 1; nproc < 8; nproc *= 2)
			fork();
		for (;;) {
			sprintf(ttyname, "/dev/tty%d", rand() % 8);
			close(open(ttyname, O_RDONLY));
			ioctl(fd, VT_GETSTATE, &state);
		}
	}

KASAN report:

	BUG: KASAN: use-after-free in vt_in_use drivers/tty/vt/vt_ioctl.c:48 [inline]
	BUG: KASAN: use-after-free in vt_ioctl+0x1ad3/0x1d70 drivers/tty/vt/vt_ioctl.c:657
	Read of size 4 at addr ffff888065722468 by task syz-vt2/132

	CPU: 0 PID: 132 Comm: syz-vt2 Not tainted 5.6.0-rc5-00130-g089b6d3654916 #13
	Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20191223_100556-anatol 04/01/2014
	Call Trace:
	 [...]
	 vt_in_use drivers/tty/vt/vt_ioctl.c:48 [inline]
	 vt_ioctl+0x1ad3/0x1d70 drivers/tty/vt/vt_ioctl.c:657
	 tty_ioctl+0x9db/0x11b0 drivers/tty/tty_io.c:2660
	 [...]

	Allocated by task 136:
	 [...]
	 kzalloc include/linux/slab.h:669 [inline]
	 alloc_tty_struct+0x96/0x8a0 drivers/tty/tty_io.c:2982
	 tty_init_dev+0x23/0x350 drivers/tty/tty_io.c:1334
	 tty_open_by_driver drivers/tty/tty_io.c:1987 [inline]
	 tty_open+0x3ca/0xb30 drivers/tty/tty_io.c:2035
	 [...]

	Freed by task 41:
	 [...]
	 kfree+0xbf/0x200 mm/slab.c:3757
	 free_tty_struct+0x8d/0xb0 drivers/tty/tty_io.c:177
	 release_one_tty+0x22d/0x2f0 drivers/tty/tty_io.c:1468
	 process_one_work+0x7f1/0x14b0 kernel/workqueue.c:2264
	 worker_thread+0x8b/0xc80 kernel/workqueue.c:2410
	 [...]

Fixes: 4001d7b7fc27 ("vt: push down the tty lock so we can see what is left to tackle")
Cc: <stable@vger.kernel.org> # v3.4+
Acked-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20200322034305.210082-3-ebiggers@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/tty/vt/vt_ioctl.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -43,9 +43,15 @@ bool vt_dont_switch;
 
 static inline bool vt_in_use(unsigned int i)
 {
-	extern struct tty_driver *console_driver;
+	const struct vc_data *vc = vc_cons[i].d;
 
-	return console_driver->ttys[i] && console_driver->ttys[i]->count;
+	/*
+	 * console_lock must be held to prevent the vc from being deallocated
+	 * while we're checking whether it's in-use.
+	 */
+	WARN_CONSOLE_UNLOCKED();
+
+	return vc && kref_read(&vc->port.kref) > 1;
 }
 
 static inline bool vt_busy(int i)
@@ -643,15 +649,16 @@ int vt_ioctl(struct tty_struct *tty,
 		struct vt_stat __user *vtstat = up;
 		unsigned short state, mask;
 
-		/* Review: FIXME: Console lock ? */
 		if (put_user(fg_console + 1, &vtstat->v_active))
 			ret = -EFAULT;
 		else {
 			state = 1;	/* /dev/tty0 is always open */
+			console_lock(); /* required by vt_in_use() */
 			for (i = 0, mask = 2; i < MAX_NR_CONSOLES && mask;
 							++i, mask <<= 1)
 				if (vt_in_use(i))
 					state |= mask;
+			console_unlock();
 			ret = put_user(state, &vtstat->v_state);
 		}
 		break;
@@ -661,10 +668,11 @@ int vt_ioctl(struct tty_struct *tty,
 	 * Returns the first available (non-opened) console.
 	 */
 	case VT_OPENQRY:
-		/* FIXME: locking ? - but then this is a stupid API */
+		console_lock(); /* required by vt_in_use() */
 		for (i = 0; i < MAX_NR_CONSOLES; ++i)
 			if (!vt_in_use(i))
 				break;
+		console_unlock();
 		uival = i < MAX_NR_CONSOLES ? (i+1) : -1;
 		goto setint;		 
 



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

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 16:17 [PATCH 5.5 00/30] 5.5.15-rc1 review Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 01/30] bpf: update jmp32 test cases to fix range bound deduction Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 02/30] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 03/30] mac80211: fix authentication with iwlwifi/mvm Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 04/30] serial: sprd: Fix a dereference warning Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 05/30] vt: selection, introduce vc_is_sel Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 06/30] vt: ioctl, switch VT_IS_IN_USE and VT_BUSY to inlines Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 07/30] vt: switch vt_dont_switch to bool Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 08/30] vt: vt_ioctl: remove unnecessary console allocation checks Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 09/30] vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console Greg Kroah-Hartman
2020-04-01 16:17 ` Greg Kroah-Hartman [this message]
2020-04-01 16:17 ` [PATCH 5.5 11/30] platform/x86: pmc_atom: Add Lex 2I385SW to critclk_systems DMI table Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 12/30] bpf: Explicitly memset the bpf_attr structure Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 13/30] bpf: Explicitly memset some bpf info structures declared on the stack Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 14/30] iwlwifi: dont send GEO_TX_POWER_LIMIT if no wgds table Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 15/30] 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.5 16/30] net: ks8851-ml: Fix IO operations, again Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 17/30] clk: imx: Align imx sc clock msg structs to 4 Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 18/30] clk: imx: Align imx sc clock parent " Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 19/30] clk: ti: am43xx: Fix clock parent for RTC clock Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 20/30] libceph: fix alloc_msg_with_page_vector() memory leaks Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 21/30] arm64: alternative: fix build with clang integrated assembler Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 22/30] perf map: Fix off by one in strncpy() size argument Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 23/30] ARM: dts: oxnas: Fix clear-mask property Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 24/30] ARM: dts: bcm283x: Fix vc4s firmware bus DMA limitations Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 25/30] ARM: bcm2835-rpi-zero-w: Add missing pinctrl name Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 26/30] ARM: dts: imx6: phycore-som: fix arm and soc minimum voltage Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 27/30] ARM: dts: N900: fix onenand timings Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 28/30] ARM: dts: sun8i: r40: Move AHCI device node based on address order Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 29/30] arm64: dts: ls1043a-rdb: correct RGMII delay mode to rgmii-id Greg Kroah-Hartman
2020-04-01 16:17 ` [PATCH 5.5 30/30] arm64: dts: ls1046ardb: set RGMII interfaces to RGMII_ID mode Greg Kroah-Hartman
2020-04-02  0:13 ` [PATCH 5.5 00/30] 5.5.15-rc1 review Guenter Roeck
2020-04-02  7:10 ` Jon Hunter
2020-04-02  9:29 ` Naresh Kamboju
2020-04-02 16:45 ` shuah
2020-04-02 18:43   ` 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=20200401161423.206522162@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 \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).