From 180c67540cd933a75fbbccf47a6ad7f9f4ff1ce5 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 1 Apr 2020 16:03:53 +0200 Subject: [PATCH] 5.6-stable patches added patches: platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch serial-sprd-fix-a-dereference-warning.patch vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch vt-selection-introduce-vc_is_sel.patch vt-switch-vt_dont_switch-to-bool.patch vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch --- ...2i385sw-to-critclk_systems-dmi-table.patch | 46 +++++ ...erial-sprd-fix-a-dereference-warning.patch | 44 +++++ queue-5.6/series | 8 + ...-vt_is_in_use-and-vt_busy-to-inlines.patch | 87 +++++++++ .../vt-selection-introduce-vc_is_sel.patch | 101 ++++++++++ .../vt-switch-vt_dont_switch-to-bool.patch | 57 ++++++ ...octl-fix-use-after-free-in-vt_in_use.patch | 138 ++++++++++++++ ...ocate-freeing-in-use-virtual-console.patch | 175 ++++++++++++++++++ ...nnecessary-console-allocation-checks.patch | 68 +++++++ 9 files changed, 724 insertions(+) create mode 100644 queue-5.6/platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch create mode 100644 queue-5.6/serial-sprd-fix-a-dereference-warning.patch create mode 100644 queue-5.6/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch create mode 100644 queue-5.6/vt-selection-introduce-vc_is_sel.patch create mode 100644 queue-5.6/vt-switch-vt_dont_switch-to-bool.patch create mode 100644 queue-5.6/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch create mode 100644 queue-5.6/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch create mode 100644 queue-5.6/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch diff --git a/queue-5.6/platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch b/queue-5.6/platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch new file mode 100644 index 00000000000..81362d81524 --- /dev/null +++ b/queue-5.6/platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch @@ -0,0 +1,46 @@ +From 95b31e35239e5e1689e3d965d692a313c71bd8ab Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Georg=20M=C3=BCller?= +Date: Mon, 3 Feb 2020 21:11:06 +0100 +Subject: platform/x86: pmc_atom: Add Lex 2I385SW to critclk_systems DMI table +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Georg Müller + +commit 95b31e35239e5e1689e3d965d692a313c71bd8ab upstream. + +The Lex 2I385SW board has two Intel I211 ethernet controllers. Without +this patch, only the first port is usable. The second port fails to +start with the following message: + + igb: probe of 0000:02:00.0 failed with error -2 + +Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") +Tested-by: Georg Müller +Signed-off-by: Georg Müller +Reviewed-by: Hans de Goede +Signed-off-by: Andy Shevchenko +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/platform/x86/pmc_atom.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +--- a/drivers/platform/x86/pmc_atom.c ++++ b/drivers/platform/x86/pmc_atom.c +@@ -385,6 +385,14 @@ static const struct dmi_system_id critcl + }, + { + /* pmc_plt_clk* - are used for ethernet controllers */ ++ .ident = "Lex 2I385SW", ++ .matches = { ++ DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"), ++ DMI_MATCH(DMI_PRODUCT_NAME, "2I385SW"), ++ }, ++ }, ++ { ++ /* pmc_plt_clk* - are used for ethernet controllers */ + .ident = "Beckhoff CB3163", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"), diff --git a/queue-5.6/serial-sprd-fix-a-dereference-warning.patch b/queue-5.6/serial-sprd-fix-a-dereference-warning.patch new file mode 100644 index 00000000000..5fdc3dd6b9b --- /dev/null +++ b/queue-5.6/serial-sprd-fix-a-dereference-warning.patch @@ -0,0 +1,44 @@ +From efc176929a3505a30c3993ddd393b40893649bd2 Mon Sep 17 00:00:00 2001 +From: Lanqing Liu +Date: Mon, 16 Mar 2020 11:13:33 +0800 +Subject: serial: sprd: Fix a dereference warning + +From: Lanqing Liu + +commit efc176929a3505a30c3993ddd393b40893649bd2 upstream. + +We should validate if the 'sup' is NULL or not before freeing DMA +memory, to fix below warning. + +"drivers/tty/serial/sprd_serial.c:1141 sprd_remove() + error: we previously assumed 'sup' could be null (see line 1132)" + +Fixes: f4487db58eb7 ("serial: sprd: Add DMA mode support") +Reported-by: Dan Carpenter +Signed-off-by: Lanqing Liu +Cc: stable +Link: https://lore.kernel.org/r/e2bd92691538e95b04a2c2a728f3292e1617018f.1584325957.git.liuhhome@gmail.com +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/tty/serial/sprd_serial.c | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +--- a/drivers/tty/serial/sprd_serial.c ++++ b/drivers/tty/serial/sprd_serial.c +@@ -1132,14 +1132,13 @@ static int sprd_remove(struct platform_d + if (sup) { + uart_remove_one_port(&sprd_uart_driver, &sup->port); + sprd_port[sup->port.line] = NULL; ++ sprd_rx_free_buf(sup); + sprd_ports_num--; + } + + if (!sprd_ports_num) + uart_unregister_driver(&sprd_uart_driver); + +- sprd_rx_free_buf(sup); +- + return 0; + } + diff --git a/queue-5.6/series b/queue-5.6/series index 1b0b9fb4ce1..be65e273570 100644 --- a/queue-5.6/series +++ b/queue-5.6/series @@ -1,2 +1,10 @@ bpf-update-jmp32-test-cases-to-fix-range-bound-deduction.patch mac80211-fix-authentication-with-iwlwifi-mvm.patch +serial-sprd-fix-a-dereference-warning.patch +vt-selection-introduce-vc_is_sel.patch +vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch +vt-switch-vt_dont_switch-to-bool.patch +vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch +vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch +vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch +platform-x86-pmc_atom-add-lex-2i385sw-to-critclk_systems-dmi-table.patch diff --git a/queue-5.6/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch b/queue-5.6/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch new file mode 100644 index 00000000000..f1dd8f04786 --- /dev/null +++ b/queue-5.6/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch @@ -0,0 +1,87 @@ +From e587e8f17433ddb26954f0edf5b2f95c42155ae9 Mon Sep 17 00:00:00 2001 +From: Jiri Slaby +Date: Wed, 19 Feb 2020 08:39:44 +0100 +Subject: vt: ioctl, switch VT_IS_IN_USE and VT_BUSY to inlines + +From: Jiri Slaby + +commit e587e8f17433ddb26954f0edf5b2f95c42155ae9 upstream. + +These two were macros. Switch them to static inlines, so that it's more +understandable what they are doing. + +Signed-off-by: Jiri Slaby +Link: https://lore.kernel.org/r/20200219073951.16151-2-jslaby@suse.cz +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/tty/vt/vt_ioctl.c | 29 ++++++++++++++++++++++------- + 1 file changed, 22 insertions(+), 7 deletions(-) + +--- a/drivers/tty/vt/vt_ioctl.c ++++ b/drivers/tty/vt/vt_ioctl.c +@@ -40,10 +40,25 @@ + #include + + 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_BUSY(i) (VT_IS_IN_USE(i) || i == fg_console || vc_is_sel(vc_cons[i].d)) ++static inline bool vt_in_use(unsigned int i) ++{ ++ extern struct tty_driver *console_driver; ++ ++ return console_driver->ttys[i] && console_driver->ttys[i]->count; ++} ++ ++static inline bool vt_busy(int i) ++{ ++ if (vt_in_use(i)) ++ return true; ++ if (i == fg_console) ++ return true; ++ if (vc_is_sel(vc_cons[i].d)) ++ return true; ++ ++ return false; ++} + + /* + * Console (vt and kd) routines, as defined by USL SVR4 manual, and by +@@ -289,7 +304,7 @@ static int vt_disallocate(unsigned int v + int ret = 0; + + console_lock(); +- if (VT_BUSY(vc_num)) ++ if (vt_busy(vc_num)) + ret = -EBUSY; + else if (vc_num) + vc = vc_deallocate(vc_num); +@@ -311,7 +326,7 @@ static void vt_disallocate_all(void) + + console_lock(); + for (i = 1; i < MAX_NR_CONSOLES; i++) +- if (!VT_BUSY(i)) ++ if (!vt_busy(i)) + vc[i] = vc_deallocate(i); + else + vc[i] = NULL; +@@ -648,7 +663,7 @@ int vt_ioctl(struct tty_struct *tty, + state = 1; /* /dev/tty0 is always open */ + for (i = 0, mask = 2; i < MAX_NR_CONSOLES && mask; + ++i, mask <<= 1) +- if (VT_IS_IN_USE(i)) ++ if (vt_in_use(i)) + state |= mask; + ret = put_user(state, &vtstat->v_state); + } +@@ -661,7 +676,7 @@ int vt_ioctl(struct tty_struct *tty, + case VT_OPENQRY: + /* FIXME: locking ? - but then this is a stupid API */ + for (i = 0; i < MAX_NR_CONSOLES; ++i) +- if (! VT_IS_IN_USE(i)) ++ if (!vt_in_use(i)) + break; + uival = i < MAX_NR_CONSOLES ? (i+1) : -1; + goto setint; diff --git a/queue-5.6/vt-selection-introduce-vc_is_sel.patch b/queue-5.6/vt-selection-introduce-vc_is_sel.patch new file mode 100644 index 00000000000..fdaa5d8eaa9 --- /dev/null +++ b/queue-5.6/vt-selection-introduce-vc_is_sel.patch @@ -0,0 +1,101 @@ +From dce05aa6eec977f1472abed95ccd71276b9a3864 Mon Sep 17 00:00:00 2001 +From: Jiri Slaby +Date: Wed, 19 Feb 2020 08:39:43 +0100 +Subject: vt: selection, introduce vc_is_sel + +From: Jiri Slaby + +commit dce05aa6eec977f1472abed95ccd71276b9a3864 upstream. + +Avoid global variables (namely sel_cons) by introducing vc_is_sel. It +checks whether the parameter is the current selection console. This will +help putting sel_cons to a struct later. + +Signed-off-by: Jiri Slaby +Link: https://lore.kernel.org/r/20200219073951.16151-1-jslaby@suse.cz +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/tty/vt/selection.c | 5 +++++ + drivers/tty/vt/vt.c | 7 ++++--- + drivers/tty/vt/vt_ioctl.c | 2 +- + include/linux/selection.h | 4 +++- + 4 files changed, 13 insertions(+), 5 deletions(-) + +--- a/drivers/tty/vt/selection.c ++++ b/drivers/tty/vt/selection.c +@@ -88,6 +88,11 @@ void clear_selection(void) + } + EXPORT_SYMBOL_GPL(clear_selection); + ++bool vc_is_sel(struct vc_data *vc) ++{ ++ return vc == sel_cons; ++} ++ + /* + * User settable table: what characters are to be considered alphabetic? + * 128 bits. Locked by the console lock. +--- a/drivers/tty/vt/vt.c ++++ b/drivers/tty/vt/vt.c +@@ -890,8 +890,9 @@ static void hide_softcursor(struct vc_da + + static void hide_cursor(struct vc_data *vc) + { +- if (vc == sel_cons) ++ if (vc_is_sel(vc)) + clear_selection(); ++ + vc->vc_sw->con_cursor(vc, CM_ERASE); + hide_softcursor(vc); + } +@@ -901,7 +902,7 @@ static void set_cursor(struct vc_data *v + if (!con_is_fg(vc) || console_blanked || vc->vc_mode == KD_GRAPHICS) + return; + if (vc->vc_deccm) { +- if (vc == sel_cons) ++ if (vc_is_sel(vc)) + clear_selection(); + add_softcursor(vc); + if ((vc->vc_cursor_type & 0x0f) != 1) +@@ -1207,7 +1208,7 @@ static int vc_do_resize(struct tty_struc + } + } + +- if (vc == sel_cons) ++ if (vc_is_sel(vc)) + clear_selection(); + + old_rows = vc->vc_rows; +--- a/drivers/tty/vt/vt_ioctl.c ++++ b/drivers/tty/vt/vt_ioctl.c +@@ -43,7 +43,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_BUSY(i) (VT_IS_IN_USE(i) || i == fg_console || vc_cons[i].d == sel_cons) ++#define VT_BUSY(i) (VT_IS_IN_USE(i) || i == fg_console || vc_is_sel(vc_cons[i].d)) + + /* + * Console (vt and kd) routines, as defined by USL SVR4 manual, and by +--- a/include/linux/selection.h ++++ b/include/linux/selection.h +@@ -11,8 +11,8 @@ + #include + #include + +-extern struct vc_data *sel_cons; + struct tty_struct; ++struct vc_data; + + extern void clear_selection(void); + extern int set_selection_user(const struct tiocl_selection __user *sel, +@@ -24,6 +24,8 @@ extern int sel_loadlut(char __user *p); + extern int mouse_reporting(void); + extern void mouse_report(struct tty_struct * tty, int butt, int mrx, int mry); + ++bool vc_is_sel(struct vc_data *vc); ++ + extern int console_blanked; + + extern const unsigned char color_table[]; diff --git a/queue-5.6/vt-switch-vt_dont_switch-to-bool.patch b/queue-5.6/vt-switch-vt_dont_switch-to-bool.patch new file mode 100644 index 00000000000..17edba5e423 --- /dev/null +++ b/queue-5.6/vt-switch-vt_dont_switch-to-bool.patch @@ -0,0 +1,57 @@ +From f400991bf872debffb01c46da882dc97d7e3248e Mon Sep 17 00:00:00 2001 +From: Jiri Slaby +Date: Wed, 19 Feb 2020 08:39:48 +0100 +Subject: vt: switch vt_dont_switch to bool + +From: Jiri Slaby + +commit f400991bf872debffb01c46da882dc97d7e3248e upstream. + +vt_dont_switch is pure boolean, no need for whole char. + +Signed-off-by: Jiri Slaby +Link: https://lore.kernel.org/r/20200219073951.16151-6-jslaby@suse.cz +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/tty/vt/vt_ioctl.c | 6 +++--- + include/linux/vt_kern.h | 2 +- + 2 files changed, 4 insertions(+), 4 deletions(-) + +--- a/drivers/tty/vt/vt_ioctl.c ++++ b/drivers/tty/vt/vt_ioctl.c +@@ -39,7 +39,7 @@ + #include + #include + +-char vt_dont_switch; ++bool vt_dont_switch; + + static inline bool vt_in_use(unsigned int i) + { +@@ -1026,12 +1026,12 @@ int vt_ioctl(struct tty_struct *tty, + case VT_LOCKSWITCH: + if (!capable(CAP_SYS_TTY_CONFIG)) + return -EPERM; +- vt_dont_switch = 1; ++ vt_dont_switch = true; + break; + case VT_UNLOCKSWITCH: + if (!capable(CAP_SYS_TTY_CONFIG)) + return -EPERM; +- vt_dont_switch = 0; ++ vt_dont_switch = false; + break; + case VT_GETHIFONTMASK: + ret = put_user(vc->vc_hi_font_mask, +--- a/include/linux/vt_kern.h ++++ b/include/linux/vt_kern.h +@@ -135,7 +135,7 @@ extern int do_unbind_con_driver(const st + int deflt); + int vty_init(const struct file_operations *console_fops); + +-extern char vt_dont_switch; ++extern bool vt_dont_switch; + extern int default_utf8; + extern int global_cursor_default; + diff --git a/queue-5.6/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch b/queue-5.6/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch new file mode 100644 index 00000000000..9695185880b --- /dev/null +++ b/queue-5.6/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch @@ -0,0 +1,138 @@ +From 7cf64b18b0b96e751178b8d0505d8466ff5a448f Mon Sep 17 00:00:00 2001 +From: Eric Biggers +Date: Sat, 21 Mar 2020 20:43:05 -0700 +Subject: vt: vt_ioctl: fix use-after-free in vt_in_use() + +From: Eric Biggers + +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 + #include + + 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: # v3.4+ +Acked-by: Jiri Slaby +Signed-off-by: Eric Biggers +Link: https://lore.kernel.org/r/20200322034305.210082-3-ebiggers@kernel.org +Signed-off-by: Greg Kroah-Hartman + +--- + 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; + diff --git a/queue-5.6/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch b/queue-5.6/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch new file mode 100644 index 00000000000..0897c293fdd --- /dev/null +++ b/queue-5.6/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch @@ -0,0 +1,175 @@ +From ca4463bf8438b403596edd0ec961ca0d4fbe0220 Mon Sep 17 00:00:00 2001 +From: Eric Biggers +Date: Sat, 21 Mar 2020 20:43:04 -0700 +Subject: vt: vt_ioctl: fix VT_DISALLOCATE freeing in-use virtual console + +From: Eric Biggers + +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 + #include + #include + #include + + 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: # v3.4+ +Reported-by: syzbot+522643ab5729b0421998@syzkaller.appspotmail.com +Acked-by: Jiri Slaby +Signed-off-by: Eric Biggers +Link: https://lore.kernel.org/r/20200322034305.210082-2-ebiggers@kernel.org +Signed-off-by: Greg Kroah-Hartman + +--- + 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) +@@ -3414,7 +3434,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); + } + } + diff --git a/queue-5.6/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch b/queue-5.6/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch new file mode 100644 index 00000000000..14e20b7f500 --- /dev/null +++ b/queue-5.6/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch @@ -0,0 +1,68 @@ +From 1aa6e058dd6cd04471b1f21298270014daf48ac9 Mon Sep 17 00:00:00 2001 +From: Eric Biggers +Date: Mon, 24 Feb 2020 00:03:26 -0800 +Subject: vt: vt_ioctl: remove unnecessary console allocation checks + +From: Eric Biggers + +commit 1aa6e058dd6cd04471b1f21298270014daf48ac9 upstream. + +The vc_cons_allocated() checks in vt_ioctl() and vt_compat_ioctl() are +unnecessary because they can only be reached by calling ioctl() on an +open tty, which implies the corresponding virtual console is allocated. + +And even if the virtual console *could* be freed concurrently, then +these checks would be broken since they aren't done under console_lock, +and the vc_data is dereferenced before them anyway. + +So, remove these unneeded checks to avoid confusion. + +Signed-off-by: Eric Biggers +Link: https://lore.kernel.org/r/20200224080326.295046-1-ebiggers@kernel.org +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/tty/vt/vt_ioctl.c | 16 +--------------- + 1 file changed, 1 insertion(+), 15 deletions(-) + +--- a/drivers/tty/vt/vt_ioctl.c ++++ b/drivers/tty/vt/vt_ioctl.c +@@ -350,22 +350,13 @@ int vt_ioctl(struct tty_struct *tty, + { + struct vc_data *vc = tty->driver_data; + struct console_font_op op; /* used in multiple places here */ +- unsigned int console; ++ unsigned int console = vc->vc_num; + unsigned char ucval; + unsigned int uival; + void __user *up = (void __user *)arg; + int i, perm; + int ret = 0; + +- console = vc->vc_num; +- +- +- if (!vc_cons_allocated(console)) { /* impossible? */ +- ret = -ENOIOCTLCMD; +- goto out; +- } +- +- + /* + * To have permissions to do most of the vt ioctls, we either have + * to be the owner of the tty, or have CAP_SYS_TTY_CONFIG. +@@ -1195,14 +1186,9 @@ long vt_compat_ioctl(struct tty_struct * + { + struct vc_data *vc = tty->driver_data; + struct console_font_op op; /* used in multiple places here */ +- unsigned int console = vc->vc_num; + void __user *up = compat_ptr(arg); + int perm; + +- +- if (!vc_cons_allocated(console)) /* impossible? */ +- return -ENOIOCTLCMD; +- + /* + * To have permissions to do most of the vt ioctls, we either have + * to be the owner of the tty, or have CAP_SYS_TTY_CONFIG. -- 2.47.3