tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT

This requirement was overeagerly loosened in commit 2f83e38a095f
("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN"), but

tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT

This requirement was overeagerly loosened in commit 2f83e38a095f
("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN"), but as
it turns out,

(1) the logic I implemented there was inconsistent (apologies!),

(2) TIOCL_SELMOUSEREPORT might actually be a small security risk
after all, and

(3) TIOCL_SELMOUSEREPORT is only meant to be used by the mouse
daemon (GPM or Consolation), which runs as CAP_SYS_ADMIN
already.

In more detail:

1. The previous patch has inconsistent logic:

In commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes
without CAP_SYS_ADMIN"), we checked for sel_mode ==
TIOCL_SELMOUSEREPORT, but overlooked that the lower four bits of
this "mode" parameter were actually used as an additional way to
pass an argument. So the patch did actually still require
CAP_SYS_ADMIN, if any of the mouse button bits are set, but did not
require it if none of the mouse buttons bits are set.

This logic is inconsistent and was not intentional. We should have
the same policies for using TIOCL_SELMOUSEREPORT independent of the
value of the "hidden" mouse button argument.

I sent a separate documentation patch to the man page list with
more details on TIOCL_SELMOUSEREPORT:
https://lore.kernel.org/all/[email protected]/

2. TIOCL_SELMOUSEREPORT is indeed a potential security risk which can
let an attacker simulate "keyboard" input to command line
applications on the same terminal, like TIOCSTI and some other
TIOCLINUX "selection mode" IOCTLs.

By enabling mouse reporting on a terminal and then injecting mouse
reports through TIOCL_SELMOUSEREPORT, an attacker can simulate
mouse movements on the same terminal, similar to the TIOCSTI
keystroke injection attacks that were previously possible with
TIOCSTI and other TIOCL_SETSEL selection modes.

Many programs (including libreadline/bash) are then prone to
misinterpret these mouse reports as normal keyboard input because
they do not expect input in the X11 mouse protocol form. The
attacker does not have complete control over the escape sequence,
but they can at least control the values of two consecutive bytes
in the binary mouse reporting escape sequence.

I went into more detail on that in the discussion at
https://lore.kernel.org/all/[email protected]/

It is not equally trivial to simulate arbitrary keystrokes as it
was with TIOCSTI (commit 83efeeeb3d04 ("tty: Allow TIOCSTI to be
disabled")), but the general mechanism is there, and together with
the small number of existing legit use cases (see below), it would
be better to revert back to requiring CAP_SYS_ADMIN for
TIOCL_SELMOUSEREPORT, as it was already the case before
commit 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without
CAP_SYS_ADMIN").

3. TIOCL_SELMOUSEREPORT is only used by the mouse daemons (GPM or
Consolation), and they are the only legit use case:

To quote console_codes(4):

The mouse tracking facility is intended to return
xterm(1)-compatible mouse status reports. Because the console
driver has no way to know the device or type of the mouse, these
reports are returned in the console input stream only when the
virtual terminal driver receives a mouse update ioctl. These
ioctls must be generated by a mouse-aware user-mode application
such as the gpm(8) daemon.

Jared Finder has also confirmed in
https://lore.kernel.org/all/[email protected]/
that Emacs does not call TIOCL_SELMOUSEREPORT directly, and it
would be difficult to find good reasons for doing that, given that
it would interfere with the reports that GPM is sending.

More information on the interaction between GPM, terminals and the
kernel with additional pointers is also available in this patch:
https://lore.kernel.org/all/a773e48920aa104a65073671effbdee665c105fc.1603963593.git.tammo.block@gmail.com/

For background on who else uses TIOCL_SELMOUSEREPORT: Debian Code
search finds one page of results, the only two known callers are
the two mouse daemons GPM and Consolation. (GPM does not show up
in the search results because it uses literal numbers to refer to
TIOCLINUX-related enums. I looked through GPM by hand instead.
TIOCL_SELMOUSEREPORT is also not used from libgpm.)
https://codesearch.debian.net/search?q=TIOCL_SELMOUSEREPORT

Cc: Jared Finder <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Hanno Böck <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: stable <[email protected]>
Fixes: 2f83e38a095f ("tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN")
Signed-off-by: Günther Noack <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN

With this, processes without CAP_SYS_ADMIN are able to use TIOCLINUX with
subcode TIOCL_SETSEL, in the selection modes TIOCL_SETPOINTER,
TIO

tty: Permit some TIOCL_SETSEL modes without CAP_SYS_ADMIN

With this, processes without CAP_SYS_ADMIN are able to use TIOCLINUX with
subcode TIOCL_SETSEL, in the selection modes TIOCL_SETPOINTER,
TIOCL_SELCLEAR and TIOCL_SELMOUSEREPORT.

TIOCL_SETSEL was previously changed to require CAP_SYS_ADMIN, as this IOCTL
let callers change the selection buffer and could be used to simulate
keypresses. These three TIOCL_SETSEL selection modes, however, are safe to
use, as they do not modify the selection buffer.

This fixes a mouse support regression that affected Emacs (invisible mouse
cursor).

Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Fixes: 8d1b43f6a6df ("tty: Restrict access to TIOCLINUX' copy-and-paste subcommands")
Signed-off-by: Günther Noack <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: vt: fix up kernel-doc

selection.c and vt.c still uses tabs in the kernel-doc. This misrenders the
functions in the output -- sphinx misinterprets the description. So
remove these tabs, incl. th

tty: vt: fix up kernel-doc

selection.c and vt.c still uses tabs in the kernel-doc. This misrenders the
functions in the output -- sphinx misinterprets the description. So
remove these tabs, incl. those around dashes.

'enum' keyword is needed before enum names. Fix that.

Superfluous \n after the comments are also removed. They are not
completely faulty, but this unifies all the kernel-doc in the files.

Finally fix up the cross references.

Signed-off-by: "Jiri Slaby (SUSE)" <[email protected]>
Reviewed-by: Randy Dunlap <[email protected]>
Tested-by: Helge Deller <[email protected]> # parisc STI console
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: vt: make vc_is_sel()'s vc const

It's only an aid to people reading the header and/or calling
vc_is_sel(). vc is only tested there, so having it const makes sense.

Signed-off-by: "Jiri Slaby (S

tty: vt: make vc_is_sel()'s vc const

It's only an aid to people reading the header and/or calling
vc_is_sel(). vc is only tested there, so having it const makes sense.

Signed-off-by: "Jiri Slaby (SUSE)" <[email protected]>
Tested-by: Helge Deller <[email protected]> # parisc STI console
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: vt: push console lock from tioclinux() down to 2 functions

Avoid costly user copies under the console lock. So push the lock down
from tioclinux() to sel_loadlut() and set_vesa_blanking().

It

tty: vt: push console lock from tioclinux() down to 2 functions

Avoid costly user copies under the console lock. So push the lock down
from tioclinux() to sel_loadlut() and set_vesa_blanking().

It is now obvious what is actually protected.

Signed-off-by: "Jiri Slaby (SUSE)" <[email protected]>
Tested-by: Helge Deller <[email protected]> # parisc STI console
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: vt: pass proper pointers from tioclinux()

Pass proper types and proper pointers (the data with an offset) to the
TIOCL_* handlers. So that they need not to cast or add anything to the
passed po

tty: vt: pass proper pointers from tioclinux()

Pass proper types and proper pointers (the data with an offset) to the
TIOCL_* handlers. So that they need not to cast or add anything to the
passed pointer.

This makes obvious what is passed/consumed.

Signed-off-by: "Jiri Slaby (SUSE)" <[email protected]>
Tested-by: Helge Deller <[email protected]> # parisc STI console
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: switch count in tty_ldisc_receive_buf() to size_t

It comes from both paste_selection() and tty_port_default_receive_buf()
as unsigned (int and size_t respectively). Switch to size_t to converge

tty: switch count in tty_ldisc_receive_buf() to size_t

It comes from both paste_selection() and tty_port_default_receive_buf()
as unsigned (int and size_t respectively). Switch to size_t to converge
to that eventually.

Return the count as size_t too (the two callers above expect that).

Switch paste_selection()'s type of 'count' too, so that the returned and
passed type match.

Signed-off-by: "Jiri Slaby (SUSE)" <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty/vt: consolemap: make parameters of inverse_translate() saner

- int use_unicode -> bool: it's used as bool at some places already, so
make it explicit.
- int glyph -> u16: every caller passes a

tty/vt: consolemap: make parameters of inverse_translate() saner

- int use_unicode -> bool: it's used as bool at some places already, so
make it explicit.
- int glyph -> u16: every caller passes a u16 in. So make it explicit
too. And remove a negative check from inverse_translate() as it never
could be negative.

Reviewed-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty: vt: selection: Correct misspelled function sel_loadlut()

Fixes the following W=1 kernel build warning(s):

drivers/tty/vt/selection.c:119: warning: expecting prototype for set loadlut(). Proto

tty: vt: selection: Correct misspelled function sel_loadlut()

Fixes the following W=1 kernel build warning(s):

drivers/tty/vt/selection.c:119: warning: expecting prototype for set loadlut(). Prototype was for sel_loadlut() instead

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: Move custom isspace() to its own namespace

If by some reason any of the headers will include ctype.h
we will have a name collision. Avoid this by moving isspace()
to the dedicate namespace.

Fir

vt: Move custom isspace() to its own namespace

If by some reason any of the headers will include ctype.h
we will have a name collision. Avoid this by moving isspace()
to the dedicate namespace.

First appearance of the code is in the commit 24a1c2a769cf
("Import 1.1.92").

Reported-by: kernel test robot <[email protected]>
Reviewed-by: Jiri Slaby <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vc: propagate "viewed as bool" from screenpos up

viewed is used as a flag, i.e. bool. So treat is as such in most of the
places. vcs_vc is handled in the next patch.

Note: the last parameter of inv

vc: propagate "viewed as bool" from screenpos up

viewed is used as a flag, i.e. bool. So treat is as such in most of the
places. vcs_vc is handled in the next patch.

Note: the last parameter of invert_screen was misnamed in the
declaration since 1.1.92.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

tty/vt: Do not warn when huge selection requested

The tty TIOCL_SETSEL ioctl allocates a memory buffer big enough for text
selection area. The maximum allowed console size is
VC_RESIZE_MAXCOL * VC_R

tty/vt: Do not warn when huge selection requested

The tty TIOCL_SETSEL ioctl allocates a memory buffer big enough for text
selection area. The maximum allowed console size is
VC_RESIZE_MAXCOL * VC_RESIZE_MAXROW == 32767*32767 == ~1GB and typical
MAX_ORDER is set to allow allocations lot less than than (circa 16MB).

So it is quite possible to trigger huge allocation (and syzkaller just
did that) which is going to fail (which is fine) with a backtrace in
mm/page_alloc.c at WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN)) and
this may trigger panic (if panic_on_warn is enabled) and
leak kernel addresses to dmesg.

This passes __GFP_NOWARN to kmalloc_array to avoid unnecessary user-
triggered WARN_ON. Note that the error is not ignored and
the warning is still printed.

Signed-off-by: Alexey Kardashevskiy <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: extract selection chars storing from vc_do_selection

Let's put it to a separate function, named vc_selection_store_chars.
Again, this makes vc_do_selection a bit shorter and more readable.
Havin

vt: extract selection chars storing from vc_do_selection

Let's put it to a separate function, named vc_selection_store_chars.
Again, this makes vc_do_selection a bit shorter and more readable.
Having 4 local variables instead of 12 (5.6-rc1) looks much better now.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, split __set_selection_kernel

Handle these actions:
* poking console
* TIOCL_SELCLEAR
* TIOCL_SELMOUSEREPORT
* start/end precomputation
* clear_selection if the console changed
in a se

vt: selection, split __set_selection_kernel

Handle these actions:
* poking console
* TIOCL_SELCLEAR
* TIOCL_SELMOUSEREPORT
* start/end precomputation
* clear_selection if the console changed
in a separate function, thus making __set_selection_kernel way shorter
and more readable. The function still needs dissection, but we are
approaching.

This includes introduction of vc_selection and renaming
__set_selection_kernel to vc_do_selection.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, use rounddown() for start/endline computation

We have a helper called rounddown for these modulo computations. So use
it.

No functional change intended and "objdump -d" proves that.

vt: selection, use rounddown() for start/endline computation

We have a helper called rounddown for these modulo computations. So use
it.

No functional change intended and "objdump -d" proves that.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, fix double lock introduced by a merge

The merge commit cb05c6c82fb0 (Merge 5.6-rc5 into tty-next) introduced a
double lock to set_selection_kernel. vc_sel.lock is locked both in
set_s

vt: selection, fix double lock introduced by a merge

The merge commit cb05c6c82fb0 (Merge 5.6-rc5 into tty-next) introduced a
double lock to set_selection_kernel. vc_sel.lock is locked both in
set_selection_kernel and its callee __set_selection_kernel now.

Remove the latter.

Signed-off-by: Jiri Slaby <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, indent switch-case properly

Shift the cases one level left as this is how we are supposed to write
the switch-case code according to the CodingStyle.

Signed-off-by: Jiri Slaby <jslab

vt: selection, indent switch-case properly

Shift the cases one level left as this is how we are supposed to write
the switch-case code according to the CodingStyle.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, remove redeclaration of poke_blanked_console

It is declared in vt_kern.h, so no need to declare it in selection.c
which includes the header.

Signed-off-by: Jiri Slaby <[email protected]

vt: selection, remove redeclaration of poke_blanked_console

It is declared in vt_kern.h, so no need to declare it in selection.c
which includes the header.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, create struct from console selection globals

Move all the selection global variables to a structure vc_selection,
instantiated as vc_sel. This helps to group all the variables togethe

vt: selection, create struct from console selection globals

Move all the selection global variables to a structure vc_selection,
instantiated as vc_sel. This helps to group all the variables together
and see what should be protected by the embedded lock too.

It might be used later also for per-console selection support.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, localize use_unicode

use_unicode needs not be global. It is used only in set_selection_kernel
and sel_pos (a callee). It is also always set there prior calling
sel_pos. So make use_un

vt: selection, localize use_unicode

use_unicode needs not be global. It is used only in set_selection_kernel
and sel_pos (a callee). It is also always set there prior calling
sel_pos. So make use_unicode local and rename it to plain shorter
"unicode". Finally, propagate it to sel_pos via parameter.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, remove 2 local variables from set_selection_kernel

multiplier and mode are not actually needed:
* multiplier is used only in kmalloc_array, so use "use_unicode ? 4 : 1"
directly
* m

vt: selection, remove 2 local variables from set_selection_kernel

multiplier and mode are not actually needed:
* multiplier is used only in kmalloc_array, so use "use_unicode ? 4 : 1"
directly
* mode is used only to assign a bool in this manner:
if (cond)
x = true;
else
x = false;
So do "x = cond" directly.

Signed-off-by: Jiri Slaby <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, push sel_lock up

sel_lock cannot nest in the console lock. Thanks to syzkaller, the
kernel states firmly:

> WARNING: possible circular locking dependency detected
> 5.6.0-rc3-syzkall

vt: selection, push sel_lock up

sel_lock cannot nest in the console lock. Thanks to syzkaller, the
kernel states firmly:

> WARNING: possible circular locking dependency detected
> 5.6.0-rc3-syzkaller #0 Not tainted
> ------------------------------------------------------
> syz-executor.4/20336 is trying to acquire lock:
> ffff8880a2e952a0 (&tty->termios_rwsem){++++}, at: tty_unthrottle+0x22/0x100 drivers/tty/tty_ioctl.c:136
>
> but task is already holding lock:
> ffffffff89462e70 (sel_lock){+.+.}, at: paste_selection+0x118/0x470 drivers/tty/vt/selection.c:374
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #2 (sel_lock){+.+.}:
> mutex_lock_nested+0x1b/0x30 kernel/locking/mutex.c:1118
> set_selection_kernel+0x3b8/0x18a0 drivers/tty/vt/selection.c:217
> set_selection_user+0x63/0x80 drivers/tty/vt/selection.c:181
> tioclinux+0x103/0x530 drivers/tty/vt/vt.c:3050
> vt_ioctl+0x3f1/0x3a30 drivers/tty/vt/vt_ioctl.c:364

This is ioctl(TIOCL_SETSEL).
Locks held on the path: console_lock -> sel_lock

> -> #1 (console_lock){+.+.}:
> console_lock+0x46/0x70 kernel/printk/printk.c:2289
> con_flush_chars+0x50/0x650 drivers/tty/vt/vt.c:3223
> n_tty_write+0xeae/0x1200 drivers/tty/n_tty.c:2350
> do_tty_write drivers/tty/tty_io.c:962 [inline]
> tty_write+0x5a1/0x950 drivers/tty/tty_io.c:1046

This is write().
Locks held on the path: termios_rwsem -> console_lock

> -> #0 (&tty->termios_rwsem){++++}:
> down_write+0x57/0x140 kernel/locking/rwsem.c:1534
> tty_unthrottle+0x22/0x100 drivers/tty/tty_ioctl.c:136
> mkiss_receive_buf+0x12aa/0x1340 drivers/net/hamradio/mkiss.c:902
> tty_ldisc_receive_buf+0x12f/0x170 drivers/tty/tty_buffer.c:465
> paste_selection+0x346/0x470 drivers/tty/vt/selection.c:389
> tioclinux+0x121/0x530 drivers/tty/vt/vt.c:3055
> vt_ioctl+0x3f1/0x3a30 drivers/tty/vt/vt_ioctl.c:364

This is ioctl(TIOCL_PASTESEL).
Locks held on the path: sel_lock -> termios_rwsem

> other info that might help us debug this:
>
> Chain exists of:
> &tty->termios_rwsem --> console_lock --> sel_lock

Clearly. From the above, we have:
console_lock -> sel_lock
sel_lock -> termios_rwsem
termios_rwsem -> console_lock

Fix this by reversing the console_lock -> sel_lock dependency in
ioctl(TIOCL_SETSEL). First, lock sel_lock, then console_lock.

Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: [email protected]
Fixes: 07e6124a1a46 ("vt: selection, close sel_buffer race")
Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, push console lock down

We need to nest the console lock in sel_lock, so we have to push it down
a bit. Fortunately, the callers of set_selection_* just lock the console
lock around th

vt: selection, push console lock down

We need to nest the console lock in sel_lock, so we have to push it down
a bit. Fortunately, the callers of set_selection_* just lock the console
lock around the function call. So moving it down is easy.

In the next patch, we switch the order.

Signed-off-by: Jiri Slaby <[email protected]>
Fixes: 07e6124a1a46 ("vt: selection, close sel_buffer race")
Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, introduce vc_is_sel

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_con

vt: selection, introduce vc_is_sel

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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

vt: selection, close sel_buffer race

syzkaller reported this UAF:
BUG: KASAN: use-after-free in n_tty_receive_buf_common+0x2481/0x2940 drivers/tty/n_tty.c:1741
Read of size 1 at addr ffff8880089e40e

vt: selection, close sel_buffer race

syzkaller reported this UAF:
BUG: KASAN: use-after-free in n_tty_receive_buf_common+0x2481/0x2940 drivers/tty/n_tty.c:1741
Read of size 1 at addr ffff8880089e40e9 by task syz-executor.1/13184

CPU: 0 PID: 13184 Comm: syz-executor.1 Not tainted 5.4.7 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
Call Trace:
...
kasan_report+0xe/0x20 mm/kasan/common.c:634
n_tty_receive_buf_common+0x2481/0x2940 drivers/tty/n_tty.c:1741
tty_ldisc_receive_buf+0xac/0x190 drivers/tty/tty_buffer.c:461
paste_selection+0x297/0x400 drivers/tty/vt/selection.c:372
tioclinux+0x20d/0x4e0 drivers/tty/vt/vt.c:3044
vt_ioctl+0x1bcf/0x28d0 drivers/tty/vt/vt_ioctl.c:364
tty_ioctl+0x525/0x15a0 drivers/tty/tty_io.c:2657
vfs_ioctl fs/ioctl.c:47 [inline]

It is due to a race between parallel paste_selection (TIOCL_PASTESEL)
and set_selection_user (TIOCL_SETSEL) invocations. One uses sel_buffer,
while the other frees it and reallocates a new one for another
selection. Add a mutex to close this race.

The mutex takes care properly of sel_buffer and sel_buffer_lth only. The
other selection global variables (like sel_start, sel_end, and sel_cons)
are protected only in set_selection_user. The other functions need quite
some more work to close the races of the variables there. This is going
to happen later.

This likely fixes (I am unsure as there is no reproducer provided) bug
206361 too. It was marked as CVE-2020-8648.

Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: [email protected]
References: https://bugzilla.kernel.org/show_bug.cgi?id=206361
Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...