tty: Fix ->session locking
authorJann Horn <jannh@google.com>
Thu, 3 Dec 2020 01:25:05 +0000 (02:25 +0100)
committerPDO SCM Team <hudsoncm@motorola.com>
Fri, 1 Oct 2021 05:15:24 +0000 (00:15 -0500)
commitfb3999102be662a76907255d0ca0f18f6861f3af
tree6d84ef009e584bf4d74775012b0f053c1e770c14
parenta9b207466f86bdeb2a67d397d5c70cb1c681e4f0
tty: Fix ->session locking

commit c8bcd9c5be24fb9e6132e97da5a35e55a83e36b9 upstream.

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().

Mot-CRs-fixed: (CR)
CVE-Fixed: CVE-2020-29660
Bug: 175451844

Change-Id: I9f630624459fe567925513161a694d49a201a8ea
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>
(cherry picked from commit adee91d5b2f80be95c94ead6d53fcfe6c4731b90)
Signed-off-by: Gajjala Chakradhar <gajjalac@motorola.com>
Reviewed-on: https://gerrit.mot.com/2071080
SME-Granted: SME Approvals Granted
SLTApproved: Slta Waiver
Tested-by: Jira Key
Reviewed-by: Xiangpo Zhao <zhaoxp3@motorola.com>
Submit-Approved: Jira Key
(cherry picked from commit 5fcb95f834a1ead2328626b53cf907f6ddbbebf7)
drivers/tty/tty_io.c
drivers/tty/tty_jobctrl.c
include/linux/tty.h