linux-stable.git/drivers/tty/tty_io.c, branch v5.11 Linux kernel stable tree tty: avoid using vfs_iocb_iter_write() for redirected console writes 2021-01-29T21:12:17+00:00 Linus Torvalds torvalds@linux-foundation.org 2021-01-29T20:28:20+00:00 a9cbbb80e3e7dd38ceac166e0698f161862a18ae It turns out that the vfs_iocb_iter_{read,write}() functions are entirely broken, and don't actually use the passed-in file pointer for IO - only for the preparatory work (permission checking and for the write_iter function lookup). That worked fine for overlayfs, which always builds the new iocb with the same file pointer that it passes in, but in the general case it ends up doing nonsensical things (and could cause an iterator call that doesn't even match the passed-in file pointer). This subtly broke the tty conversion to write_iter in commit 9bb48c82aced ("tty: implement write_iter"), because the console redirection didn't actually end up redirecting anything, since the passed-in file pointer was basically ignored, and the actual write was done with the original non-redirected console tty after all. The main visible effect of this is that the console messages were no longer logged to /var/log/boot.log during graphical boot. Fix the issue by simply not using the vfs write "helper" function at all, and just redirecting the write entirely internally to the tty layer. Do the target writability permission checks when actually registering the target tty with TIOCCONS instead of at write time. Fixes: 9bb48c82aced ("tty: implement write_iter") Reported-and-tested-by: Hans de Goede <hdegoede@redhat.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: stable@kernel.org Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> 
It turns out that the vfs_iocb_iter_{read,write}() functions are
entirely broken, and don't actually use the passed-in file pointer for
IO - only for the preparatory work (permission checking and for the
write_iter function lookup).

That worked fine for overlayfs, which always builds the new iocb with
the same file pointer that it passes in, but in the general case it ends
up doing nonsensical things (and could cause an iterator call that
doesn't even match the passed-in file pointer).

This subtly broke the tty conversion to write_iter in commit
9bb48c82aced ("tty: implement write_iter"), because the console
redirection didn't actually end up redirecting anything, since the
passed-in file pointer was basically ignored, and the actual write was
done with the original non-redirected console tty after all.

The main visible effect of this is that the console messages were no
longer logged to /var/log/boot.log during graphical boot.

Fix the issue by simply not using the vfs write "helper" function at
all, and just redirecting the write entirely internally to the tty
layer.  Do the target writability permission checks when actually
registering the target tty with TIOCCONS instead of at write time.

Fixes: 9bb48c82aced ("tty: implement write_iter")
Reported-and-tested-by: Hans de Goede <hdegoede@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Commit 9bb48c82aced ("tty: implement write_iter") converted the tty 2021-01-25T20:08:07+00:00 Sami Tolvanen samitolvanen@google.com 2021-01-25T19:09:25+00:00 9f12e37cae44a96132fc3031535a0b165486941a layer to use write_iter. Fix the redirected_tty_write declaration also in n_tty and change the comparisons to use write_iter instead of write. [ Also moved the declaration of redirected_tty_write() to the proper location in a header file. The reason for the bug was the bogus extern declaration in n_tty.c silently not matching the changed definition in tty_io.c, and because it wasn't in a shared header file, there was no cross-checking of the declaration. Sami noticed because Clang's Control Flow Integrity checking ended up incidentally noticing the inconsistent declaration. - Linus ] Fixes: 9bb48c82aced ("tty: implement write_iter") Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> 
layer to use write_iter. Fix the redirected_tty_write declaration
also in n_tty and change the comparisons to use write_iter instead of
write.

[ Also moved the declaration of redirected_tty_write() to the proper
  location in a header file. The reason for the bug was the bogus extern
  declaration in n_tty.c silently not matching the changed definition in
  tty_io.c, and because it wasn't in a shared header file, there was no
  cross-checking of the declaration.

  Sami noticed because Clang's Control Flow Integrity checking ended up
  incidentally noticing the inconsistent declaration.    - Linus ]

Fixes: 9bb48c82aced ("tty: implement write_iter")
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
tty: fix up hung_up_tty_write() conversion 2021-01-22T10:01:55+00:00 Linus Torvalds torvalds@linux-foundation.org 2021-01-21T18:04:27+00:00 17749851eb9ca2298e7c3b81aae4228961b36f28 In commit "tty: implement write_iter", I left the write_iter conversion of the hung up tty case alone, because I incorrectly thought it didn't matter. Jiri showed me the errors of my ways, and pointed out the problems with that incomplete conversion. Fix it all up. Reported-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Reviewed-by: Jiri Slaby <jirislaby@kernel.org> Link: https://lore.kernel.org/r/CAHk-=wh+-rGsa=xruEWdg_fJViFG8rN9bpLrfLz=_yBYh2tBhA@mail.gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
In commit "tty: implement write_iter", I left the write_iter conversion
of the hung up tty case alone, because I incorrectly thought it didn't
matter.

Jiri showed me the errors of my ways, and pointed out the problems with
that incomplete conversion.  Fix it all up.

Reported-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/CAHk-=wh+-rGsa=xruEWdg_fJViFG8rN9bpLrfLz=_yBYh2tBhA@mail.gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Merge 9bb48c82aced ("tty: implement write_iter") into tty-linus 2021-01-21T08:38:37+00:00 Greg Kroah-Hartman gregkh@linuxfoundation.org 2021-01-21T08:37:00+00:00 494e63ee9c9f322c1051acc537ac5b99688f7e58 We want the single "splice/sendfile to a tty" regression fix into tty-linus so it can get into 5.11-final, while the larger patch series fixing "splice/sendfile from a tty" should wait for 5.12-rc1 so that we get more testing. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
We want the single "splice/sendfile to a tty" regression fix into
tty-linus so it can get into 5.11-final, while the larger patch series
fixing "splice/sendfile from a tty" should wait for 5.12-rc1 so that we
get more testing.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: implement write_iter 2021-01-21T00:48:47+00:00 Linus Torvalds torvalds@linux-foundation.org 2021-01-19T19:41:16+00:00 9bb48c82aced07698a2d08ee0f1475a6c4f6b266 This makes the tty layer use the .write_iter() function instead of the traditional .write() functionality. That allows writev(), but more importantly also makes it possible to enable .splice_write() for ttys, reinstating the "splice to tty" functionality that was lost in commit 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") Reported-by: Oliver Giles <ohw.giles@gmail.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> 
This makes the tty layer use the .write_iter() function instead of the
traditional .write() functionality.

That allows writev(), but more importantly also makes it possible to
enable .splice_write() for ttys, reinstating the "splice to tty"
functionality that was lost in commit 36e2c7421f02 ("fs: don't allow
splice read/write without explicit ops").

Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
Reported-by: Oliver Giles <ohw.giles@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Merge 5.10-rc7 into tty-next 2020-12-07T09:19:31+00:00 Greg Kroah-Hartman gregkh@linuxfoundation.org 2020-12-07T09:19:31+00:00 ba3b8bb1263dfb87914ef0ccf5f3a78604b3686b We want the tty fixes in here as well. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
We want the tty fixes in here as well.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: Fix ->session locking 2020-12-04T16:39:58+00:00 Jann Horn jannh@google.com 2020-12-03T01:25:05+00:00 c8bcd9c5be24fb9e6132e97da5a35e55a83e36b9 Currently, locking of ->session is very inconsistent; most places protect it using the legacy tty mutex, but disassociate_ctty(), __do_SAK(), tiocspgrp() and tiocgsid() don't. Two of the writers hold the ctrl_lock (because they already need it for ->pgrp), but __proc_set_tty() doesn't do that yet. On a PREEMPT=y system, an unprivileged user can theoretically abuse this broken locking to read 4 bytes of freed memory via TIOCGSID if tiocgsid() is preempted long enough at the right point. (Other things might also go wrong, especially if root-only ioctls are involved; I'm not sure about that.) Change the locking on ->session such that: - tty_lock() is held by all writers: By making disassociate_ctty() hold it. This should be fine because the same lock can already be taken through the call to tty_vhangup_session(). The tricky part is that we need to shorten the area covered by siglock to be able to take tty_lock() without ugly retry logic; as far as I can tell, this should be fine, since nothing in the signal_struct is touched in the `if (tty)` branch. - ctrl_lock is held by all writers: By changing __proc_set_tty() to hold the lock a little longer. - All readers that aren't holding tty_lock() hold ctrl_lock: By adding locking to tiocgsid() and __do_SAK(), and expanding the area covered by ctrl_lock in tiocspgrp(). Cc: stable@kernel.org Signed-off-by: Jann Horn <jannh@google.com> Reviewed-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Currently, locking of ->session is very inconsistent; most places
protect it using the legacy tty mutex, but disassociate_ctty(),
__do_SAK(), tiocspgrp() and tiocgsid() don't.
Two of the writers hold the ctrl_lock (because they already need it for
->pgrp), but __proc_set_tty() doesn't do that yet.

On a PREEMPT=y system, an unprivileged user can theoretically abuse
this broken locking to read 4 bytes of freed memory via TIOCGSID if
tiocgsid() is preempted long enough at the right point. (Other things
might also go wrong, especially if root-only ioctls are involved; I'm
not sure about that.)

Change the locking on ->session such that:

 - tty_lock() is held by all writers: By making disassociate_ctty()
   hold it. This should be fine because the same lock can already be
   taken through the call to tty_vhangup_session().
   The tricky part is that we need to shorten the area covered by
   siglock to be able to take tty_lock() without ugly retry logic; as
   far as I can tell, this should be fine, since nothing in the
   signal_struct is touched in the `if (tty)` branch.
 - ctrl_lock is held by all writers: By changing __proc_set_tty() to
   hold the lock a little longer.
 - All readers that aren't holding tty_lock() hold ctrl_lock: By
   adding locking to tiocgsid() and __do_SAK(), and expanding the area
   covered by ctrl_lock in tiocspgrp().

Cc: stable@kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Merge 5.10-rc3 into tty-next 2020-11-09T09:37:10+00:00 Greg Kroah-Hartman gregkh@linuxfoundation.org 2020-11-09T09:37:10+00:00 9b085d6e889076928d307f05cabf76e35db1e63b We need the TTY/vt/serial fixes in here as well. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
We need the TTY/vt/serial fixes in here as well.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: fix crash in release_tty if tty->port is not set 2020-11-06T16:24:49+00:00 Matthias Reichl hias@horus.com 2020-11-05T12:34:32+00:00 4466d6d2f80c1193e0845d110277c56da77a6418 Commit 2ae0b31e0face ("tty: don't crash in tty_init_dev when missing tty_port") didn't fully prevent the crash as the cleanup path in tty_init_dev() calls release_tty() which dereferences tty->port without checking it for non-null. Add tty->port checks to release_tty to avoid the kernel crash. Fixes: 2ae0b31e0face ("tty: don't crash in tty_init_dev when missing tty_port") Signed-off-by: Matthias Reichl <hias@horus.com> Link: https://lore.kernel.org/r/20201105123432.4448-1-hias@horus.com Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Commit 2ae0b31e0face ("tty: don't crash in tty_init_dev when missing
tty_port") didn't fully prevent the crash as the cleanup path in
tty_init_dev() calls release_tty() which dereferences tty->port
without checking it for non-null.

Add tty->port checks to release_tty to avoid the kernel crash.

Fixes: 2ae0b31e0face ("tty: don't crash in tty_init_dev when missing tty_port")
Signed-off-by: Matthias Reichl <hias@horus.com>
Link: https://lore.kernel.org/r/20201105123432.4448-1-hias@horus.com
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tty: tty_io: Fix some kernel-doc issues 2020-11-06T09:49:27+00:00 Lee Jones lee.jones@linaro.org 2020-11-04T19:35:25+00:00 08aa5042d22bb66c90e1833f2119f272863dec87 Demote non-conformant headers and supply some missing descriptions. Fixes the following W=1 kernel build warning(s): drivers/tty/tty_io.c:218: warning: Function parameter or member 'file' not described in 'tty_free_file' drivers/tty/tty_io.c:566: warning: Function parameter or member 'exit_session' not described in '__tty_hangup' drivers/tty/tty_io.c:1077: warning: Function parameter or member 'tty' not described in 'tty_send_xchar' drivers/tty/tty_io.c:1077: warning: Function parameter or member 'ch' not described in 'tty_send_xchar' drivers/tty/tty_io.c:1155: warning: Function parameter or member 'file' not described in 'tty_driver_lookup_tty' drivers/tty/tty_io.c:1508: warning: Function parameter or member 'tty' not described in 'release_tty' drivers/tty/tty_io.c:1508: warning: Function parameter or member 'idx' not described in 'release_tty' drivers/tty/tty_io.c:2973: warning: Function parameter or member 'driver' not described in 'alloc_tty_struct' drivers/tty/tty_io.c:2973: warning: Function parameter or member 'idx' not described in 'alloc_tty_struct' Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Cc: Nick Holloway <alfie@dcs.warwick.ac.uk> Cc: -- <julian@uhunix.uhcc.hawaii.edu> Cc: Marko Kohtala <Marko.Kohtala@hut.fi> Cc: Bill Hawes <whawes@star.net> Cc: "C. Scott Ananian" <cananian@alumni.princeton.edu> Cc: Russell King <rmk@arm.linux.org.uk> Cc: Andrew Morton <andrewm@uow.edu.eu> Signed-off-by: Lee Jones <lee.jones@linaro.org> Link: https://lore.kernel.org/r/20201104193549.4026187-13-lee.jones@linaro.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
Demote non-conformant headers and supply some missing descriptions.

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

 drivers/tty/tty_io.c:218: warning: Function parameter or member 'file' not described in 'tty_free_file'
 drivers/tty/tty_io.c:566: warning: Function parameter or member 'exit_session' not described in '__tty_hangup'
 drivers/tty/tty_io.c:1077: warning: Function parameter or member 'tty' not described in 'tty_send_xchar'
 drivers/tty/tty_io.c:1077: warning: Function parameter or member 'ch' not described in 'tty_send_xchar'
 drivers/tty/tty_io.c:1155: warning: Function parameter or member 'file' not described in 'tty_driver_lookup_tty'
 drivers/tty/tty_io.c:1508: warning: Function parameter or member 'tty' not described in 'release_tty'
 drivers/tty/tty_io.c:1508: warning: Function parameter or member 'idx' not described in 'release_tty'
 drivers/tty/tty_io.c:2973: warning: Function parameter or member 'driver' not described in 'alloc_tty_struct'
 drivers/tty/tty_io.c:2973: warning: Function parameter or member 'idx' not described in 'alloc_tty_struct'

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Nick Holloway <alfie@dcs.warwick.ac.uk>
Cc: -- <julian@uhunix.uhcc.hawaii.edu>
Cc: Marko Kohtala <Marko.Kohtala@hut.fi>
Cc: Bill Hawes <whawes@star.net>
Cc: "C. Scott Ananian" <cananian@alumni.princeton.edu>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Andrew Morton <andrewm@uow.edu.eu>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Link: https://lore.kernel.org/r/20201104193549.4026187-13-lee.jones@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>