1. Patchew
  2. linux
  3. View series

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

Morduan Zang posted 1 patch 5 days, 21 hours ago 
drivers/tty/vt/keyboard.c | 17 ++++++++++++-----
drivers/tty/vt/vt.c       |  4 ++--
2 files changed, 14 insertions(+), 7 deletions(-)
[PATCH] tty: vt: hold tty reference for keyboard callbacks
Posted by Morduan Zang 5 days, 21 hours ago 
From: Zhan Jun <zhanjun@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+2932e8970a6398db95c3@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a1dde0d.bd48a97d.14881d.0005.GAE@google.com/
Signed-off-by: Zhan Jun <zhanjun@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
Re: [PATCH] tty: vt: hold tty reference for keyboard callbacks
Posted by Jiri Slaby 4 days, 21 hours ago 
On 02. 06. 26, 8:15, Morduan Zang wrote:
> From: Zhan Jun <zhanjun@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+2932e8970a6398db95c3@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6a1dde0d.bd48a97d.14881d.0005.GAE@google.com/
> Signed-off-by: Zhan Jun <zhanjun@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);

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

thanks,
-- 
js
suse labs

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.