From: Greg Kroah-Hartman Date: Wed, 1 Apr 2020 14:02:10 +0000 (+0200) Subject: 4.9-stable patches X-Git-Tag: v5.6.2~12 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ee2f38ba7826d69d9aa3e9bba738990719bbb595;p=thirdparty%2Fkernel%2Fstable-queue.git 4.9-stable patches added patches: bpf-explicitly-memset-the-bpf_attr-structure.patch locking-atomic-kref-add-kref_read.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 --- diff --git a/queue-4.9/bpf-explicitly-memset-the-bpf_attr-structure.patch b/queue-4.9/bpf-explicitly-memset-the-bpf_attr-structure.patch new file mode 100644 index 00000000000..f0fe4217559 --- /dev/null +++ b/queue-4.9/bpf-explicitly-memset-the-bpf_attr-structure.patch @@ -0,0 +1,54 @@ +From 8096f229421f7b22433775e928d506f0342e5907 Mon Sep 17 00:00:00 2001 +From: Greg Kroah-Hartman +Date: Fri, 20 Mar 2020 10:48:13 +0100 +Subject: bpf: Explicitly memset the bpf_attr structure +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Greg Kroah-Hartman + +commit 8096f229421f7b22433775e928d506f0342e5907 upstream. + +For the bpf syscall, we are relying on the compiler to properly zero out +the bpf_attr union that we copy userspace data into. Unfortunately that +doesn't always work properly, padding and other oddities might not be +correctly zeroed, and in some tests odd things have been found when the +stack is pre-initialized to other values. + +Fix this by explicitly memsetting the structure to 0 before using it. + +Reported-by: Maciej Żenczykowski +Reported-by: John Stultz +Reported-by: Alexander Potapenko +Reported-by: Alistair Delva +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Daniel Borkmann +Acked-by: Yonghong Song +Link: https://android-review.googlesource.com/c/kernel/common/+/1235490 +Link: https://lore.kernel.org/bpf/20200320094813.GA421650@kroah.com +Signed-off-by: Greg Kroah-Hartman + +--- + kernel/bpf/syscall.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +--- a/kernel/bpf/syscall.c ++++ b/kernel/bpf/syscall.c +@@ -802,7 +802,7 @@ static int bpf_obj_get(const union bpf_a + + SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) + { +- union bpf_attr attr = {}; ++ union bpf_attr attr; + int err; + + if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN)) +@@ -838,6 +838,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf + } + + /* copy attributes from user space, may be less than sizeof(bpf_attr) */ ++ memset(&attr, 0, sizeof(attr)); + if (copy_from_user(&attr, uattr, size) != 0) + return -EFAULT; + diff --git a/queue-4.9/locking-atomic-kref-add-kref_read.patch b/queue-4.9/locking-atomic-kref-add-kref_read.patch new file mode 100644 index 00000000000..9ff261099db --- /dev/null +++ b/queue-4.9/locking-atomic-kref-add-kref_read.patch @@ -0,0 +1,49 @@ +From 2c935bc57221cc2edc787c72ea0e2d30cdcd3d5e Mon Sep 17 00:00:00 2001 +From: Peter Zijlstra +Date: Mon, 14 Nov 2016 17:29:48 +0100 +Subject: locking/atomic, kref: Add kref_read() + +From: Peter Zijlstra + +commit 2c935bc57221cc2edc787c72ea0e2d30cdcd3d5e upstream. + +Since we need to change the implementation, stop exposing internals. + +Provide kref_read() to read the current reference count; typically +used for debug messages. + +Kills two anti-patterns: + + atomic_read(&kref->refcount) + kref->refcount.counter + +Signed-off-by: Peter Zijlstra (Intel) +Cc: Andrew Morton +Cc: Greg Kroah-Hartman +Cc: Linus Torvalds +Cc: Paul E. McKenney +Cc: Peter Zijlstra +Cc: Thomas Gleixner +Cc: linux-kernel@vger.kernel.org +Signed-off-by: Ingo Molnar +[only add kref_read() to kref.h for stable backports - gregkh] +Signed-off-by: Greg Kroah-Hartman + +--- + include/linux/kref.h | 5 +++++ + 1 file changed, 5 insertions(+) + +--- a/include/linux/kref.h ++++ b/include/linux/kref.h +@@ -33,6 +33,11 @@ static inline void kref_init(struct kref + atomic_set(&kref->refcount, 1); + } + ++static inline int kref_read(const struct kref *kref) ++{ ++ return atomic_read(&kref->refcount); ++} ++ + /** + * kref_get - increment refcount for object. + * @kref: object. diff --git a/queue-4.9/series b/queue-4.9/series index 12312981331..c92398b059b 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -89,3 +89,11 @@ media-stv06xx-add-missing-descriptor-sanity-checks.patch media-xirlink_cit-add-missing-descriptor-sanity-checks.patch mac80211-check-port-authorization-in-the-ieee80211_tx_dequeue-case.patch mac80211-fix-authentication-with-iwlwifi-mvm.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 +locking-atomic-kref-add-kref_read.patch +vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch +bpf-explicitly-memset-the-bpf_attr-structure.patch diff --git a/queue-4.9/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch b/queue-4.9/vt-ioctl-switch-vt_is_in_use-and-vt_busy-to-inlines.patch new file mode 100644 index 00000000000..9c92df4fe87 --- /dev/null +++ b/queue-4.9/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 +@@ -39,10 +39,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 +@@ -292,7 +307,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); +@@ -314,7 +329,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; +@@ -651,7 +666,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); + } +@@ -664,7 +679,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-4.9/vt-selection-introduce-vc_is_sel.patch b/queue-4.9/vt-selection-introduce-vc_is_sel.patch new file mode 100644 index 00000000000..565d7b32619 --- /dev/null +++ b/queue-4.9/vt-selection-introduce-vc_is_sel.patch @@ -0,0 +1,102 @@ +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 +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 +@@ -80,6 +80,11 @@ void clear_selection(void) + } + } + ++bool vc_is_sel(struct vc_data *vc) ++{ ++ return vc == sel_cons; ++} ++ + /* + * User settable table: what characters are to be considered alphabetic? + * 256 bits. Locked by the console lock. +--- a/drivers/tty/vt/vt.c ++++ b/drivers/tty/vt/vt.c +@@ -595,8 +595,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); + } +@@ -606,7 +607,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) +@@ -876,7 +877,7 @@ static int vc_do_resize(struct tty_struc + if (!newscreen) + return -ENOMEM; + +- 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 +@@ -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_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 +@@ -12,8 +12,8 @@ + + struct tty_struct; + +-extern struct vc_data *sel_cons; + struct tty_struct; ++struct vc_data; + + extern void clear_selection(void); + extern int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *tty); +@@ -22,6 +22,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-4.9/vt-switch-vt_dont_switch-to-bool.patch b/queue-4.9/vt-switch-vt_dont_switch-to-bool.patch new file mode 100644 index 00000000000..5e40c7390bd --- /dev/null +++ b/queue-4.9/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 +@@ -38,7 +38,7 @@ + #include + #include + +-char vt_dont_switch; ++bool vt_dont_switch; + + static inline bool vt_in_use(unsigned int i) + { +@@ -1029,12 +1029,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 +@@ -141,7 +141,7 @@ static inline bool vt_force_oops_output( + return false; + } + +-extern char vt_dont_switch; ++extern bool vt_dont_switch; + extern int default_utf8; + extern int global_cursor_default; + diff --git a/queue-4.9/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch b/queue-4.9/vt-vt_ioctl-fix-use-after-free-in-vt_in_use.patch new file mode 100644 index 00000000000..49d13898a9c --- /dev/null +++ b/queue-4.9/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 +@@ -42,9 +42,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) +@@ -646,15 +652,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; +@@ -664,10 +671,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-4.9/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch b/queue-4.9/vt-vt_ioctl-fix-vt_disallocate-freeing-in-use-virtual-console.patch new file mode 100644 index 00000000000..dda247da056 --- /dev/null +++ b/queue-4.9/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 +@@ -754,6 +754,17 @@ static void visual_init(struct vc_data * + vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row; + } + ++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; +@@ -779,6 +790,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); +@@ -2897,6 +2909,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; +@@ -2932,6 +2945,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) +@@ -3056,7 +3076,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 +@@ -313,10 +313,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; + } +@@ -336,10 +334,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-4.9/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch b/queue-4.9/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch new file mode 100644 index 00000000000..44a3eda7b64 --- /dev/null +++ b/queue-4.9/vt-vt_ioctl-remove-unnecessary-console-allocation-checks.patch @@ -0,0 +1,81 @@ +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 | 21 ++------------------- + 1 file changed, 2 insertions(+), 19 deletions(-) + +--- a/drivers/tty/vt/vt_ioctl.c ++++ b/drivers/tty/vt/vt_ioctl.c +@@ -353,22 +353,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. +@@ -1202,18 +1193,10 @@ 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; + void __user *up = (void __user *)arg; + int 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. +@@ -1273,7 +1256,7 @@ long vt_compat_ioctl(struct tty_struct * + arg = (unsigned long)compat_ptr(arg); + goto fallback; + } +-out: ++ + return ret; + + fallback: