vt: keyboard: add NULL check for vc_cons[fg_console].d in kbd_keycode and kbd_rawcode
PATCH 1/1 vt: keyboard: add NULL check for vc_cons[fg_console].d in kbd_keycode and kbd_rawcode
kbd_keycode() and kbd_rawcode() dereference vc_cons[fg_console].d without checking if it is NULL. The foreground console should normally always be allocated, but there could be a time during console setup or teardown where this pointer could be NULL, leading to a general protection fault.
Syzkaller triggers this by injecting USB HID input events that reach kbd_event() while the console state may not be fully consistent. The crash manifests as a null-ptr-deref in __queue_work when put_queue() or puts_queue() calls tty_flip_buffer_push() on the uninitialized vc port.
Add a NULL check for vc at the start of both kbd_rawcode() and kbd_keycode() to bail out early if the foreground console is not allocated.
--- drivers/tty/vt/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/tty/vt/keyboard.c +6 -0
@@ -1389,6 +1389,9 @@ static void kbd_rawcode(unsigned char data) {struct vc_data *vc = vc_cons[fg_console].d; +if (!vc) +return ; + kbd = &kbd_table[vc->vc_num];if (kbd->kbdmode ==VC_RAW )put_queue (vc, data);
@@ -1405,6 +1408,9 @@ 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; +if (!vc) +return ; + tty = vc->port.tty;if (tty && (!tty->driver_data)) {
Signature · 1 line
2.52.0
Greg Kroah-Hartman
maintainerQuoted 29 lines · Daniel Hodges
kbd_keycode()andkbd_rawcode()dereference vc_cons[fg_console].d without checking if it is NULL. The foreground console should normally always be allocated, but there could be a time during console setup or teardown where this pointer could be NULL, leading to a general protection fault.Syzkaller triggers this by injecting USB HID input events that reach
kbd_event()while the console state may not be fully consistent. The crash manifests as a null-ptr-deref in__queue_workwhenput_queue()orputs_queue()callstty_flip_buffer_push()on the uninitialized vc port.Add a NULL check for vc at the start of both
kbd_rawcode()andkbd_keycode()to bail out early if the foreground console is not allocated.Reported-by [email protected]Reported-by [email protected]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Signed-off-by Daniel Hodges <[email protected]>--- drivers/tty/vt/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/tty/vt/keyboard.c +0
-0
drivers/tty/vt/keyboard.c static void kbd_rawcode(unsigned char data) · line 1394
Quoted 6 lines
drivers/tty/vt/keyboard.c +3
-0@@ -1389,6 +1389,9 @@ static void kbd_rawcode(unsigned char data) {struct vc_data *vc = vc_cons[fg_console].d; +if (!vc) +return ; +
What prevents vc from being NULL right after checking this?
drivers/tty/vt/keyboard.c static void kbd_keycode(unsigned int keycode, int down, bool hw_raw) · line 1412
Quoted 9 lines
drivers/tty/vt/keyboard.c +2
-0kbd = &kbd_table[vc->vc_num];if (kbd->kbdmode ==VC_RAW )put_queue (vc, data);@@ -1405,6 +1408,9 @@ 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; +if (!vc) +return ;
Same here, where is the locking?
thanks,
greg k-h
Quoted 33 lines · Greg Kroah-Hartman
Quoted 32 lines · Daniel Hodges
kbd_keycode()andkbd_rawcode()dereference vc_cons[fg_console].d without checking if it is NULL. The foreground console should normally always be allocated, but there could be a time during console setup or teardown where this pointer could be NULL, leading to a general protection fault.Syzkaller triggers this by injecting USB HID input events that reach
kbd_event()while the console state may not be fully consistent. The crash manifests as a null-ptr-deref in__queue_workwhenput_queue()orputs_queue()callstty_flip_buffer_push()on the uninitialized vc port.Add a NULL check for vc at the start of both
kbd_rawcode()andkbd_keycode()to bail out early if the foreground console is not allocated.Reported-by [email protected]Reported-by [email protected]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Signed-off-by Daniel Hodges <[email protected]>--- drivers/tty/vt/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/tty/vt/keyboard.c +0
-0@@ -1389,6 +1389,9 @@ static void kbd_rawcode(unsigned char data) {struct vc_data *vc = vc_cons[fg_console].d;
Quoted 5 lines
drivers/tty/vt/keyboard.c static void kbd_rawcode(unsigned char data) · line 1394
Quoted 3 lines
drivers/tty/vt/keyboard.c +3
-0+if (!vc) +return ; +What prevents vc from being NULL right after checking this?
Yeah, your right about that. I spent a bit of time to make a reproducer using a kernel module and I think if RCU is used on vc_cons[].d it should then be properly protected. Let me know if that sounds like a reasonable approach and I can send a v2.
Quoted 10 lines
Quoted 10 lines
drivers/tty/vt/keyboard.c +0
-0kbd = &kbd_table[vc->vc_num];if (kbd->kbdmode ==VC_RAW )put_queue (vc, data);@@ -1405,6 +1408,9 @@ 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;
Quoted 8 lines
drivers/tty/vt/keyboard.c static void kbd_keycode(unsigned int keycode, int down, bool hw_raw) · line 1412
Quoted 2 lines
drivers/tty/vt/keyboard.c +2
-0+if (!vc) +return ;Same here, where is the locking?
thanks,
greg k-h
Same as above if this is protected with RCU I think that should work properly.
-Daniel
Greg Kroah-Hartman
maintainerQuoted 40 lines · Daniel Hodges
Quoted 33 lines · Greg Kroah-Hartman
Quoted 32 lines · Daniel Hodges
kbd_keycode()andkbd_rawcode()dereference vc_cons[fg_console].d without checking if it is NULL. The foreground console should normally always be allocated, but there could be a time during console setup or teardown where this pointer could be NULL, leading to a general protection fault.Syzkaller triggers this by injecting USB HID input events that reach
kbd_event()while the console state may not be fully consistent. The crash manifests as a null-ptr-deref in__queue_workwhenput_queue()orputs_queue()callstty_flip_buffer_push()on the uninitialized vc port.Add a NULL check for vc at the start of both
kbd_rawcode()andkbd_keycode()to bail out early if the foreground console is not allocated.Reported-by [email protected]Reported-by [email protected]Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Signed-off-by Daniel Hodges <[email protected]>--- drivers/tty/vt/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/tty/vt/keyboard.c +0
-0@@ -1389,6 +1389,9 @@ static void kbd_rawcode(unsigned char data) {struct vc_data *vc = vc_cons[fg_console].d;Quoted 5 lines
drivers/tty/vt/keyboard.c static void kbd_rawcode(unsigned char data) · line 1394
Quoted 3 lines
drivers/tty/vt/keyboard.c +3
-0+if (!vc) +return ; +What prevents vc from being NULL right after checking this?
Quoted 4 lines
Yeah, your right about that. I spent a bit of time to make a reproducer using a kernel module and I think if RCU is used on vc_cons[].d it should then be properly protected. Let me know if that sounds like a reasonable approach and I can send a v2.
Maybe? But can this really ever be hit on a non-syzkaller workload?
thanks,
greg k-h