[PATCH] tty: vt: hold tty reference for keyboard callbacks

[Morduan Zang]

From: Zhan Jun <zha...@uniontech.com>

syzbot reported a use-after-free in stop_tty() when the VT
keyboard path handles the hold key.

The keyboard event path reads vc->port.tty under kbd_event_lock,
but con_shutdown() clears the pointer under console_lock and the tty
can be released after the final close. The keyboard lock therefore
does not protect the tty lifetime.

Let the VT port own a tty reference by using tty_port_tty_set() when
installing and shutting down the console tty. Use tty_port_tty_get()
in the keyboard paths before dereferencing vc->port.tty and drop the
reference after the last use.

Reported-by: syzbot+2932e8...@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a1dde0d.bd48a97...@google.com/
Signed-off-by: Zhan Jun <zha...@uniontech.com>
---
drivers/tty/vt/keyboard.c | 17 ++++++++++++-----
drivers/tty/vt/vt.c | 4 ++--
2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index dfdea0842149..19f8df9706ee 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -509,9 +509,13 @@ static void fn_show_ptregs(struct vc_data *vc)

static void fn_hold(struct vc_data *vc)
{
- struct tty_struct *tty = vc->port.tty;
+ struct tty_struct *tty;
+
+ if (rep)
+ return;

- if (rep || !tty)
+ tty = tty_port_tty_get(&vc->port);
+ if (!tty)
return;

/*
@@ -523,6 +527,8 @@ static void fn_hold(struct vc_data *vc)
start_tty(tty);
else
stop_tty(tty);
+
+ tty_kref_put(tty);
}

static void fn_num(struct vc_data *vc)
@@ -1431,9 +1437,8 @@ 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;
-
- if (tty && (!tty->driver_data)) {
+ tty = tty_port_tty_get(&vc->port);
+ if (tty && !tty->driver_data) {
/* No driver data? Strange. Okay we fix it then. */
tty->driver_data = vc;
}
@@ -1486,6 +1491,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
if (rep &&
(!vc_kbd_mode(kbd, VC_REPEAT) ||
(tty && !L_ECHO(tty) && tty_chars_in_buffer(tty)))) {
+ tty_kref_put(tty);
/*
* Don't repeat a key if the input buffers are not empty and the
* characters get aren't echoed locally. This makes key repeat
@@ -1493,6 +1499,7 @@ static void kbd_keycode(unsigned int keycode, int down, bool hw_raw)
*/
return;
}
+ tty_kref_put(tty);

param.shift = shift_final = (shift_state | kbd->slockstate) ^ kbd->lockstate;
param.ledstate = kbd->ledflagstate;
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index e99636ab9db5..ae191a1eaa05 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3755,7 +3755,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) {
@@ -3788,7 +3788,7 @@ static void con_shutdown(struct tty_struct *tty)
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.50.1

[Jiri Slaby]

We have guards (tty_port_tty) and you should use those.

thanks,
--
js
suse labs