[PATCH RFC] vt: tty: use krefs to fix a potential UAF between kbd_keycode and con_shutdown

[Wentao Guan]

syzbot report an KASAN: slab-use-after-free Read in kbd_event (2),
which allocated by alloc_tty_struct->tty_init_dev, accessed by kbd_keycode,
released by tty_release_struct:
tty_release_struct->release_tty->tty->ops->shutdown->con_shutdown.
accessed by
kbd_keycode drivers/tty/vt/keyboard.c:1435
kbd_event+0x3330/0x40d0 drivers/tty/vt/keyboard.c:1515

Use tty_port_tty_get to get a tty ref in kbd_keycode to prevent the UAF,
tty_release_struct use console_lock not protect access tty_struct from
kbd_keycode or another function, so convert it to tty_port_tty_set in
con_install, con_shutdown.

The change is similar as
commit 4a90f09b20f4622dcbff1f0e1e6bae1704f8ad8c ("tty: usb-serial krefs").

Maybe reproduce in:
CPU A		CPU B		CPU C
open /dev/tty
		close /dev/tty
				tty!=NULL
		release_tty()
				access tty_struct

Cc: stable@xxxxxxxxxx
Reported-by: syzbot+098cefc0911c68db5dab@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzbot.org/bug?extid=098cefc0911c68db5dab
Reported-by: syzbot+702b7f311487703dbb18@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzbot.org/bug?extid=702b7f311487703dbb18
Signed-off-by: Wentao Guan <guanwentao@xxxxxxxxxxxxx>
---
 drivers/tty/vt/keyboard.c | 20 +++++++++++++-------
 drivers/tty/vt/vt.c       |  5 ++---
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 13bc048f45e86..173c447525ff8 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -517,6 +517,10 @@ static void fn_hold(struct vc_data *vc)
 	 * Note: SCROLLOCK will be set (cleared) by stop_tty (start_tty);
 	 * these routines are also activated by ^S/^Q.
 	 * (And SCROLLOCK can also be set by the ioctl KDSKBLED.)
+	 *
+	 * kbd_keycode(), only from kbd_keycode via k_handler[], already holds a
+	 * reference to the tty via tty_port_tty_get(), so we can safely
+	 * access port->tty here without an extra kref.
 	 */
 	if (tty->flow.stopped)
 		start_tty(tty);
@@ -1378,7 +1382,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 	struct keyboard_notifier_param param = { .vc = vc, .value = keycode, .down = down };
 	int rc;
 
-	tty = vc->port.tty;
+	tty = tty_port_tty_get(&vc->port);
 
 	if (tty && (!tty->driver_data)) {
 		/* No driver data? Strange. Okay we fix it then. */
@@ -1438,7 +1442,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 		 * characters get aren't echoed locally. This makes key repeat
 		 * usable with slow applications and under heavy loads.
 		 */
-		return;
+		goto out;
 	}
 
 	param.shift = shift_final = (shift_state | kbd->slockstate) ^ kbd->lockstate;
@@ -1452,7 +1456,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 					   KBD_UNBOUND_KEYCODE, &param);
 		do_compute_shiftstate();
 		kbd->slockstate = 0;
-		return;
+		goto out;
 	}
 
 	if (keycode < NR_KEYS)
@@ -1460,7 +1464,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 	else if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
 		keysym = U(K(KT_BRL, keycode - KEY_BRL_DOT1 + 1));
 	else
-		return;
+		goto out;
 
 	type = KTYP(keysym);
 
@@ -1471,7 +1475,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 		if (rc != NOTIFY_STOP)
 			if (down && !(raw_mode || kbd->kbdmode == VC_OFF))
 				k_unicode(vc, keysym, !down);
-		return;
+		goto out;
 	}
 
 	type -= 0xf0;
@@ -1489,10 +1493,10 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 	rc = atomic_notifier_call_chain(&keyboard_notifier_list,
 					KBD_KEYSYM, &param);
 	if (rc == NOTIFY_STOP)
-		return;
+		goto out;
 
 	if ((raw_mode || kbd->kbdmode == VC_OFF) && type != KT_SPEC && type != KT_SHIFT)
-		return;
+		goto out;
 
 	(*k_handler[type])(vc, KVAL(keysym), !down);
 
@@ -1501,6 +1505,8 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
 
 	if (type != KT_SLOCK)
 		kbd->slockstate = 0;
+out:
+	tty_kref_put(tty);
 }
 
 static void kbd_event(struct input_handle *handle, unsigned int event_type,
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index e2df99e3d4580..acded112cae2b 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3661,7 +3661,7 @@ static int con_install(struct tty_driver *driver, struct tty_struct *tty)
 		return ret;
 
 	tty->driver_data = vc;
-	vc->port.tty = tty;
+	tty_port_tty_set(&vc->port, tty);
 	tty_port_get(&vc->port);
 
 	if (!tty->winsize.ws_row && !tty->winsize.ws_col) {
@@ -3693,8 +3693,7 @@ static void con_shutdown(struct tty_struct *tty)
 	struct vc_data *vc = tty->driver_data;
 	BUG_ON(vc == NULL);
 
-	guard(console_lock)();
-	vc->port.tty = NULL;
+	tty_port_tty_set(&vc->port, NULL);
 }
 
 static void con_cleanup(struct tty_struct *tty)
-- 
2.30.2