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;
next prev parent reply other threads:[~2020-04-01 16:20 UTC|newest]
Thread overview: 37+ 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 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 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.