xsrc/56415: wskbd(4) mangled due to WSKBD_RAW after Xorg server crash

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>Number:         56415
>Category:       xsrc
>Synopsis:       wskbd(4) mangled due to WSKBD_RAW after Xorg server crash
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    xsrc-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Sep 22 13:55:00 +0000 2021
>Originator:     Izumi Tsutsui
>Release:        NetBSD 9.2
>Organization:
>Environment:
System: NetBSD lunatic 9.2 NetBSD 9.2 (GENERIC) #0: Wed May 12 13:15:55 UTC 2021  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/luna68k/compile/GENERIC luna68k

Architecture: m68k, but maybe MI
Machine: luna68k, but may affects all ports that use WSDISPLAY_COMPAT_RAWKBD
>Description:
On ports that use WSDISPLAY_COMPAT_RAWKBD to use faked PS/2 keycode
on Xorg server, xf86-input-keyboard driver calls WSKBDIO_SETMODE to
set WSKBD_RAW mode.

However, upstream Xorg server has been changed on crash to call
a new AbortDevices() rather than CloseDownDevices() since
xorg-server 1.14.0:
 https://gitlab.freedesktop.org/xorg/xserver/-/commit/9f79e93b6b3416055d08a0e8f9f16d5fd0649e36

>> If we're about to abort, we're already in the signal handler and cannot call
>> down to the default device cleanup routines (which reset, free, alloc, and
>> do a bunch of other things).
>>
>> Add a new DEVICE_ABORT mode to signal a driver's DeviceProc that it must
>> reset the hardware if needed but do nothing else. An actual HW reset is only
>> required for some drivers dealing with the HW directly.
>> 
>> This is largely backwards-compatible, hence the input ABI minor bump only.

So if we need to restore certain driver's state (like WSKBDIO_SETMODE
for wskbd) changed on Xserver startup after crash, it looks we have
to handle it in DEVICE_ABORT in the device_control function
(usually fooProc()) in each driver.

>> Drivers we care about either return BadValue on a mode that's not
>> DEVICE_{INIT|ON|OFF|CLOSE} or print an error and return BadValue. Exception
>> here is vmmouse, which currently ignores it and would not reset anything.
>> This should be fixed if the reset is required.

Note xf86-input-keyboard KbdProc() returns BadValue on DEIVCE_ABORT.

>How-To-Repeat:
- Start Xorg server on ports that use WSDISPLAY_COMPAT_RAWKBD
  (most Tier-II ports that don't use PS/2 keyboard; hp300, luna68k, zaurus..)
- kill -SEGV [X pid]
- type keyboard on ttyE0 console
 -> We have to kill login shell from another terminal for recovery
    (or call ioctl WSKBDIO_SETMODE)

>Fix:
The following patch that adds DEVICE_ABORT to call Kbd_Off() works around
(at least on NetBSD/luna68k 9.2):

Index: external/mit/xf86-input-keyboard/dist/src/kbd.c
===================================================================
RCS file: /cvsroot/xsrc/external/mit/xf86-input-keyboard/dist/src/kbd.c,v
retrieving revision 1.7
diff -u -p -d -r1.7 kbd.c
--- external/mit/xf86-input-keyboard/dist/src/kbd.c	5 Mar 2017 08:05:23 -0000	1.7
+++ external/mit/xf86-input-keyboard/dist/src/kbd.c	22 Sep 2021 13:29:50 -0000
@@ -384,6 +384,13 @@ KbdProc(DeviceIntPtr device, int what)
     device->public.on = FALSE;
     break;
 
+  case DEVICE_ABORT:
+    /*
+     * Restore original keyboard state even on crash.
+     */
+    pKbd->KbdOff(pInfo, what);
+    break;
+    
   default:
     return BadValue;
   }

---
Izumi Tsutsui