NetBSD Problem Report #56415

From tsutsui@ceres.dti.ne.jp  Wed Sep 22 13:50:59 2021
Return-Path: <tsutsui@ceres.dti.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E1BDA1A921F
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 22 Sep 2021 13:50:59 +0000 (UTC)
Message-Id: <202109221350.18MDooWS009980@ceres.dti.ne.jp>
Date: Wed, 22 Sep 2021 22:50:50 +0900 (JST)
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Reply-To: tsutsui@ceres.dti.ne.jp
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: wskbd(4) mangled due to WSKBD_RAW after Xorg server crash
X-Send-Pr-Version: 3.95

>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:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Sep 22 13:55:00 +0000 2021
>Closed-Date:    Sun Oct 10 13:47:11 +0000 2021
>Last-Modified:  Sun Oct 10 13:47:11 +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:/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

>Release-Note:

>Audit-Trail:
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@netbsd.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Re: xsrc/56415: wskbd(4) mangled due to WSKBD_RAW after Xorg server
	 crash
Date: Thu, 23 Sep 2021 22:10:43 +0900

 > >Number:         56415
 > >Category:       xsrc
 > >Synopsis:       wskbd(4) mangled due to WSKBD_RAW after Xorg server crash
  :
 > >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;
 >    }

 Per xf86-input-elographics, it would be better to add XINPUT API check:
  https://nxr.netbsd.org/xref/xsrc/external/mit/xf86-input-elographics/dist/src/xf86Elo.c?a=true&r=1.7#925

 ---
 Index: 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
 --- kbd.c	5 Mar 2017 08:05:23 -0000	1.7
 +++ kbd.c	23 Sep 2021 12:52:43 -0000
 @@ -384,6 +384,15 @@ KbdProc(DeviceIntPtr device, int what)
      device->public.on = FALSE;
      break;

 +#if GET_ABI_MAJOR(ABI_XINPUT_VERSION) * 100 + GET_ABI_MINOR(ABI_XINPUT_VERSION) >= 1901
 +  case DEVICE_ABORT:
 +    /*
 +     * Restore original keyboard state even on crash.
 +     */
 +    pKbd->KbdOff(pInfo, what);
 +    break;
 +#endif
 +
    default:
      return BadValue;
    }


 ---
 Izumi Tsutsui

From: "Izumi Tsutsui" <tsutsui@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56415 CVS commit: xsrc/external/mit/xf86-input-keyboard/dist/src
Date: Sat, 2 Oct 2021 04:28:54 +0000

 Module Name:	xsrc
 Committed By:	tsutsui
 Date:		Sat Oct  2 04:28:54 UTC 2021

 Modified Files:
 	xsrc/external/mit/xf86-input-keyboard/dist/src: kbd.c

 Log Message:
 Handle DEVICE_ABORT to restore the original keyboard state after crash.

 No particular comment in PR xsrc/56415.


 To generate a diff of this commit:
 cvs rdiff -u -r1.7 -r1.8 xsrc/external/mit/xf86-input-keyboard/dist/src/kbd.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->pending-pullups
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Mon, 04 Oct 2021 15:23:31 +0000
State-Changed-Why:
[pullup-9 #1354]


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56415 CVS commit: [netbsd-9] xsrc/external/mit/xf86-input-keyboard/dist/src
Date: Mon, 4 Oct 2021 15:42:40 +0000

 Module Name:	xsrc
 Committed By:	martin
 Date:		Mon Oct  4 15:42:40 UTC 2021

 Modified Files:
 	xsrc/external/mit/xf86-input-keyboard/dist/src [netbsd-9]: kbd.c

 Log Message:
 Pull up following revision(s) (requested by tsutsui in ticket #1354):

 	external/mit/xf86-input-keyboard/dist/src/kbd.c: revision 1.8

 Handle DEVICE_ABORT to restore the original keyboard state after crash.

 No particular comment in PR xsrc/56415.


 To generate a diff of this commit:
 cvs rdiff -u -r1.7 -r1.7.4.1 \
     xsrc/external/mit/xf86-input-keyboard/dist/src/kbd.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Sun, 10 Oct 2021 13:47:11 +0000
State-Changed-Why:
Pulled up.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.