syzbot reported memory leak in tty/vt.
The problem was in VT_DISALLOCATE ioctl cmd.
After allocating unimap with PIO_UNIMAP it wasn't
freed via VT_DISALLOCATE, but vc_cons[currcons].d was
zeroed.

Reported-by: syzbot+bcc922b19ccc64240b42@syzkaller.appspotmail.com
Signed-off-by: NPavel Skripkin <paskripkin@gmail.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20210327214443.21548-1-paskripkin@gmail.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

s/spurrious/spurious/
s/worse/worst/
Acked-by: NRandy Dunlap <rdunlap@infradead.org>
Signed-off-by: NBhaskar Chowdhury <unixbhaskar@gmail.com>
Link: https://lore.kernel.org/r/20210314062716.28539-1-unixbhaskar@gmail.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Kernel documentation validator is not happy:

.../keyboard.c:2195: warning: expecting prototype for vt_get_shiftstate(). Prototype was for vt_get_shift_state() instead

This is due to typo, fix it here.
Reviewed-by: NJiri Slaby <jirislaby@kernel.org>
Signed-off-by: NAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20210303083229.75784-1-andriy.shevchenko@linux.intel.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

reset_vc() uses a "!in_interrupt()" conditional before resetting the
palettes, which is a blocking operation. Since commit
   8b6312f4 ("[PATCH] vt: refactor console SAK processing")

all calls are invoked from a workqueue process context, with the
blocking console lock always acquired.

Remove the "!in_interrupt()" check.
Signed-off-by: NAhmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: NSebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/20210208181615.381861-2-bigeasy@linutronix.deSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

This converts the keyboard_tasklet to use the new API in
commit 12cc923f ("tasklet: Introduce new initialization API")

The new API changes the argument passed to the callback function, but
fortunately the argument isn't used so it is straight forward to use
DECLARE_TASKLET_DISABLED() rather than DECLARE_TASKLET_DISABLED_OLD().
Signed-off-by: NEmil Renner Berthing <kernel@esmil.dk>
Link: https://lore.kernel.org/r/20210127164222.13220-1-kernel@esmil.dkSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Drop support for these ioctls:
* PIO_FONT, PIO_FONTX
* GIO_FONT, GIO_FONTX
* PIO_FONTRESET

As was demonstrated by commit 90bfdeef (tty: make FONTX ioctl use
the tty pointer they were actually passed), these ioctls are not used
from userspace, as:
1) they used to be broken (set up font on current console, not the open
   one) and racy (before the commit above)
2) KDFONTOP ioctl is used for years instead

Note that PIO_FONTRESET is defunct on most systems as VGA_CONSOLE is set
on them for ages. That turns on BROKEN_GRAPHICS_PROGRAMS which makes
PIO_FONTRESET just return an error.

We are removing KD_FONT_FLAG_OLD here as it was used only by these
removed ioctls. kd.h header exists both in kernel and uapi headers, so
we can remove the kernel one completely. Everyone includeing kd.h will
now automatically get the uapi one.

There are now unused definitions of the ioctl numbers and "struct
consolefontdesc" in kd.h, but as it is a uapi header, I am not touching
these.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-8-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

The constant 20 makes the font sum computation signed which can lead to
sign extensions and signed wraps. It's not much of a problem as we build
with -fno-strict-overflow. But if we ever decide not to, be ready, so
switch the constant to unsigned.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-7-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Commit 5ce2087e (Fix default compose table initialization) fixed
unicode table so that the values are not sign extended. The upstream
(kbd package) chose a different approach. They use hexadecimal values.
So use the same, so that the output of loadkeys and our shipped file
correspond more to each other.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-4-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

loadkeys (from kbd) generates 'unsigned short' instead of 'u_short'
since 2.0.3. It also marks maps as 'static' for longer than kbd's
history.

So adapt the shipped defkeymap.c to conform more to loadkeys output.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-3-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Now that the last extern user of the tasklet (set_leds) is in
keyboard.c, we can make keyboard_tasklet local to this unit too.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-2-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

set_leds and compute_shiftstate are called from a single place in vt.c.
Let's combine these two into vt_set_leds_compute_shiftstate. This allows
for making keyboard_tasklet local in the next patch.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210105120239.28031-1-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Replaces spaces with tabs for indentation.
Signed-off-by: NYan.Gao <gao.yanB@h3c.com>
Link: https://lore.kernel.org/r/20201209030551.48029-1-gao.yanB@h3c.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

We have for some time the assign_bit() API to replace open coded

	if (foo)
		set_bit(n, bar);
	else
		clear_bit(n, bar);

Use this API in VT keyboard library code.
Acked-by: NJiri Slaby <jirislaby@kernel.org>
Signed-off-by: NAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20201109105601.47159-3-andriy.shevchenko@linux.intel.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Instead of 10, 13 use \n, \r respectively.
Acked-by: NJiri Slaby <jirislaby@kernel.org>
Signed-off-by: NAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20201109105601.47159-2-andriy.shevchenko@linux.intel.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

There are few places when BIT() macro is suitable and makes code
easier to understand.
Signed-off-by: NAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20201109105601.47159-1-andriy.shevchenko@linux.intel.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

It's buggy:

On Fri, Nov 06, 2020 at 10:30:08PM +0800, Minh Yuan wrote:
> We recently discovered a slab-out-of-bounds read in fbcon in the latest
> kernel ( v5.10-rc2 for now ).  The root cause of this vulnerability is that
> "fbcon_do_set_font" did not handle "vc->vc_font.data" and
> "vc->vc_font.height" correctly, and the patch
> <https://lkml.org/lkml/2020/9/27/223> for VT_RESIZEX can't handle this
> issue.
>
> Specifically, we use KD_FONT_OP_SET to set a small font.data for tty6, and
> use  KD_FONT_OP_SET again to set a large font.height for tty1. After that,
> we use KD_FONT_OP_COPY to assign tty6's vc_font.data to tty1's vc_font.data
> in "fbcon_do_set_font", while tty1 retains the original larger
> height. Obviously, this will cause an out-of-bounds read, because we can
> access a smaller vc_font.data with a larger vc_font.height.

Further there was only one user ever.
- Android's loadfont, busybox and console-tools only ever use OP_GET
  and OP_SET
- fbset documentation only mentions the kernel cmdline font: option,
  not anything else.
- systemd used OP_COPY before release 232 published in Nov 2016

Now unfortunately the crucial report seems to have gone down with
gmane, and the commit message doesn't say much. But the pull request
hints at OP_COPY being broken

https://github.com/systemd/systemd/pull/3651

So in other words, this never worked, and the only project which
foolishly every tried to use it, realized that rather quickly too.

Instead of trying to fix security issues here on dead code by adding
missing checks, fix the entire thing by removing the functionality.

Note that systemd code using the OP_COPY function ignored the return
value, so it doesn't matter what we're doing here really - just in
case a lone server somewhere happens to be extremely unlucky and
running an affected old version of systemd. The relevant code from
font_copy_to_all_vcs() in systemd was:

	/* copy font from active VT, where the font was uploaded to */
	cfo.op = KD_FONT_OP_COPY;
	cfo.height = vcs.v_active-1; /* tty1 == index 0 */
	(void) ioctl(vcfd, KDFONTOP, &cfo);

Note this just disables the ioctl, garbage collecting the now unused
callbacks is left for -next.

v2: Tetsuo found the old mail, which allowed me to find it on another
archive. Add the link too.
Acked-by: NPeilin Ye <yepeilin.cs@gmail.com>
Reported-by: NMinh Yuan <yuanmingbuaa@gmail.com>
References: https://lists.freedesktop.org/archives/systemd-devel/2016-June/036935.html
References: https://github.com/systemd/systemd/pull/3651
Cc: Greg KH <greg@kroah.com>
Cc: Peilin Ye <yepeilin.cs@gmail.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: NDaniel Vetter <daniel.vetter@intel.com>
Link: https://lore.kernel.org/r/20201108153806.3140315-1-daniel.vetter@ffwll.chSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

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

 drivers/tty/vt/consolemap.c:739: warning: Function parameter or member 'ct' not described in 'con_get_unimap'
 drivers/tty/vt/consolemap.c:739: warning: Function parameter or member 'uct' not described in 'con_get_unimap'
 drivers/tty/vt/consolemap.c:739: warning: Function parameter or member 'list' not described in 'con_get_unimap'

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Jakub Jelinek <jj@ultra.linux.cz>
Cc: Stanislav Voronyi <stas@cnti.uanet.kharkov.ua>
Signed-off-by: NLee Jones <lee.jones@linaro.org>
Link: https://lore.kernel.org/r/20201104193549.4026187-10-lee.jones@linaro.orgSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

'puts_queue' currently loops over characters and employs the full tty
buffer machinery for every character. Do the buffer allocation only once
and copy all the character at once. This is achieved using
tty_insert_flip_string instead of loop+tty_insert_flip_char.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-17-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Instead of a 'for' loop with 'test_bit's to find a bit in a range, use
find_next_bit to achieve the same in a simpler and faster manner.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-16-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Instead of a multiline macro, convert HW_RAW to an inline function. It
allows for type checking of the parameter. And given we split the code
into two tests, it is now more readable too.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-15-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Do the permission check on a single place. That is where perm is
really checked.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-14-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Setting of function key strings is now very complex. It uses a global
buffer 'func_buf' which is prefilled in defkeymap.c_shipped. Then there
is also an index table called 'func_table'. So initially, we have
something like this:
char func_buf[] =	"\e[[A\0" // for F1
			"\e[[B\0" // for F2
			...;
char *func_table[] = {
	func_buf + 0, // for F1
	func_buf + 5, // for F2
	... }

When a user changes some specific func string by KDSKBSENT, it is
changed in 'func_buf'. If it is shorter or equal to the current one, it
is handled by a very quick 'strcpy'.

When the user's string is longer, the whole 'func_buf' is reallocated to
allow expansion somewhere in the middle. The buffer before the user's
string is copied, the user's string appended and the rest appended too.
Now, the index table (func_table) needs to be recomputed, of course.
One more complication is the held spinlock -- we have to unlock,
reallocate, lock again and do the whole thing again to be sure noone
raced with us.

In this patch, we chose completely orthogonal approach: when the user's
string is longer than the current one, we simply assign the 'kstrdup'ed
copy to the index table (func_table) and modify func_buf in no way. We
only need to make sure we free the old entries. So we need a bitmap
is_kmalloc and free the old entries (but not the original func_buf
rodata string).

Also note that we do not waste so much space as previous approach. We
only allocate space for single entries which are longer, while before,
the whole buffer was duplicated plus space for the longer string.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-12-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

KDGKBSENT (the getter) needs only 'user_kdgkb->kb_func' from the
userspace, i.e. the index. Then it needs a buffer for a local copy of
'kb_string'.

KDSKBSENT (the setter) needs a copy up to the length of
'user_kdgkb->kb_string'.

That means, we obtain the index before the switch-case and use it in
both paths and:
1) allocate full space in the getter case, and
2) copy the string only in the setter case. We do it by strndup_user
   helper now which was not available when this function was written.

Given we copy the two members of 'struct kbsentry' separately, we no
longer need a local definition. Hence we need to change all the sizeofs
here too.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-11-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

There are too many one-letter variables in vt_do_kdgkb_ioctl which is
rather confusing.  Rename 'i' to 'kb_func' and change its type to be the
same as its originating value (struct kbsentry.kb_func) -- unsigned
char.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-10-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

rep is used as a bool in the code, so declare it as such.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-9-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

key_down is sued as a bitmap using test_bit, set_bit and similar.
So declare it using DECLARE_BITMAP to make it obvious even from the
declaration.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-8-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Do the permission check on a single place. That is where perm is really
checked.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-7-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Split vt_do_kdsk_ioctl into three functions:
* getter (KDGKBENT/vt_kdgkbent)
* setter (KDSKBENT/vt_kdskbent)
* switch-case helper (vt_do_kdsk_ioctl)

This eliminates the need of ugly one-letter macros as we use parameters
now:
* i aka tmp.kb_index -> idx
* s aka tmp.kb_table -> map
* v aka tmp.kb_value -> val
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-6-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Define one limit per line and index them by their index, so that it is
clear what is what.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-5-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

There are many includes and it is hard to check if something is there or
not. So sort them alphabetically.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-3-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

We use spin locks, but don't include linux/spinlock.h in keyboards.c. So
fix this up.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-2-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

ctrl_alt_del is already declared in linux/reboot.h which we include. So
remove this second (superfluous) declaration.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201029113222.32640-1-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Some of the font tty ioctl's always used the current foreground VC for
their operations.  Don't do that then.

This fixes a data race on fg_console.

Side note: both Michael Ellerman and Jiri Slaby point out that all these
ioctls are deprecated, and should probably have been removed long ago,
and everything seems to be using the KDFONTOP ioctl instead.

In fact, Michael points out that it looks like busybox's loadfont
program seems to have switched over to using KDFONTOP exactly _because_
of this bug (ahem.. 12 years ago ;-).
Reported-by: NMinh Yuan <yuanmingbuaa@gmail.com>
Acked-by: NMichael Ellerman <mpe@ellerman.id.au>
Acked-by: NJiri Slaby <jirislaby@kernel.org>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>

In commit 5ba12787, we shuffled with the check of 'perm'. But my
brain somehow inverted the condition in 'do_unimap_ioctl' (I thought
it is ||, not &&), so GIO_UNIMAP stopped working completely.

Move the 'perm' checks back to do_unimap_ioctl and do them right again.
In fact, this reverts this part of code to the pre-5ba12787 state.
Except 'perm' is now a bool.

Fixes: 5ba12787 ("vt_ioctl: move perm checks level up")
Cc: stable@vger.kernel.org
Reported-by: NFabian Vogt <fvogt@suse.com>
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201026055419.30518-1-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Both read-side users of func_table/func_buf need locking. Without that,
one can easily confuse the code by repeatedly setting altering strings
like:
while (1)
	for (a = 0; a < 2; a++) {
		struct kbsentry kbs = {};
		strcpy((char *)kbs.kb_string, a ? ".\n" : "88888\n");
		ioctl(fd, KDSKBSENT, &kbs);
	}

When that program runs, one can get unexpected output by holding F1
(note the unxpected period on the last line):
.
88888
.8888

So protect all accesses to 'func_table' (and func_buf) by preexisting
'func_buf_lock'.

It is easy in 'k_fn' handler as 'puts_queue' is expected not to sleep.
On the other hand, KDGKBSENT needs a local (atomic) copy of the string
because copy_to_user can sleep. Use already allocated, but unused
'kbs->kb_string' for that purpose.

Note that the program above needs at least CAP_SYS_TTY_CONFIG.

This depends on the previous patch and on the func_buf_lock lock added
in commit 46ca3f73 (tty/vt: fix write/write race in ioctl(KDSKBSENT)
handler) in 5.2.

Likely fixes CVE-2020-25656.

Cc: <stable@vger.kernel.org>
Reported-by: NMinh Yuan <yuanmingbuaa@gmail.com>
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201019085517.10176-2-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Use 'strlen' of the string, add one for NUL terminator and simply do
'copy_to_user' instead of the explicit 'for' loop. This makes the
KDGKBSENT case more compact.

The only thing we need to take care about is NULL 'func_table[i]'. Use
an empty string in that case.

The original check for overflow could never trigger as the func_buf
strings are always shorter or equal to 'struct kbsentry's.

Cc: <stable@vger.kernel.org>
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20201019085517.10176-1-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for
vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than
actual font height calculated by con_font_set() from ioctl(PIO_FONT).
Since fbcon_set_font() from con_font_set() allocates minimal amount of
memory based on actual font height calculated by con_font_set(),
use of vt_resizex() can cause UAF/OOB read for font data.

VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what
comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX.

  #define VT_RESIZE   0x5609 /* set kernel's idea of screensize */
  #define VT_RESIZEX  0x560A /* set kernel's idea of screensize + more */

So far we are not aware of syzbot reports caused by setting non-zero value
to v_vlin parameter. But given that it is possible that nobody is using
VT_RESIZEX, we can try removing support for v_clin and v_vlin parameters.

Therefore, this patch effectively makes VT_RESIZEX behave like VT_RESIZE,
with emitting a message if somebody is still using v_clin and/or v_vlin
parameters.

[1] https://syzkaller.appspot.com/bug?id=32577e96d88447ded2d3b76d71254fb855245837
[2] https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3Reported-by: Nsyzbot <syzbot+b308f5fd049fbbc6e74f@syzkaller.appspotmail.com>
Reported-by: Nsyzbot <syzbot+16469b5e8e5a72e9131e@syzkaller.appspotmail.com>
Signed-off-by: NTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/4933b81b-9b1a-355b-df0e-9b31e8280ab9@i-love.sakura.ne.jpSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

This reverts commit b1c32fcf, because
Syzkaller reports a use-after-free, a write in vcs_read:

BUG: KASAN: use-after-free in vcs_read_buf drivers/tty/vt/vc_screen.c:357 [inline]
BUG: KASAN: use-after-free in vcs_read+0xaa7/0xb40 drivers/tty/vt/vc_screen.c:449
Write of size 2 at addr ffff8880a8014000 by task syz-executor.5/16936
CPU: 1 PID: 16936 Comm: syz-executor.5 Not tainted 5.9.0-rc1-next-20200820-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
...
 kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
 vcs_read_buf drivers/tty/vt/vc_screen.c:357 [inline]
 vcs_read+0xaa7/0xb40 drivers/tty/vt/vc_screen.c:449

There are two issues with the patch:
1) vcs_read rounds the 'count' *up* to an even number. So if we read odd
   bytes from the header (3 bytes in the reproducer), the second byte of
   a (2-byte/ushort) write to temporary con_buf won't fit. It is because
   with the patch applied, we only subtract the real number read (3 bytes)
   and not the whole header (4 bytes).

2) in this scenario, we perform unaligned accesses now: there are
   2-byte/ushort writes to odd addresses. Due to the same reason as
   above.

Revert this for now, re-think and retry later.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Reported-by: syzbot+ad1f53726c3bd11180cb@syzkaller.appspotmail.com
Fixes: b1c32fcf ("vc_screen: extract vcs_read_buf_header")
Cc: akpm@linux-foundation.org
Cc: nico@fluxnic.net
Link: https://lore.kernel.org/r/20200824095425.4376-1-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

Replace the existing /* fall through */ comments and its variants with
the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
fall-through markings when it is the case.

[1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-throughSigned-off-by: NGustavo A. R. Silva <gustavoars@kernel.org>

As in the previous patches, fix kernel-doc in vt.
Signed-off-by: NJiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20200818085655.12071-6-jslaby@suse.czSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>