| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <MWHPR21MB159385686CD54855A5FBE251D7E39@MWHPR21MB1593.namprd21.prod.outlook.com>
Date: Wed, 21 Jul 2021 03:19:36 +0000
From: Michael Kelley <mikelley@...rosoft.com>
To: Nathan Chancellor <nathan@...nel.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>
CC: "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
KY Srinivasan <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: RE: [PATCH] Input: serio - make write method mandatory
From: Nathan Chancellor <nathan@...nel.org> Sent: Tuesday, July 20, 2021 6:56 PM
>
> On Sun, Mar 21, 2021 at 08:53:40PM -0700, Dmitry Torokhov wrote:
> > Given that all serio drivers except one implement write() method
> > let's make it mandatory to avoid testing for its presence whenever
> > we attempt to use it.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> > ---
> > drivers/input/serio/ams_delta_serio.c | 6 ++++++
> > drivers/input/serio/serio.c | 5 +++++
> > include/linux/serio.h | 5 +----
> > 3 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> > index 1c0be299f179..a1c314897951 100644
> > --- a/drivers/input/serio/ams_delta_serio.c
> > +++ b/drivers/input/serio/ams_delta_serio.c
> > @@ -89,6 +89,11 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
> > return IRQ_HANDLED;
> > }
> >
> > +static int ams_delta_serio_write(struct serio *serio, u8 data)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > static int ams_delta_serio_open(struct serio *serio)
> > {
> > struct ams_delta_serio *priv = serio->port_data;
> > @@ -157,6 +162,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> > priv->serio = serio;
> >
> > serio->id.type = SERIO_8042;
> > + serio->write = ams_delta_serio_write;
> > serio->open = ams_delta_serio_open;
> > serio->close = ams_delta_serio_close;
> > strlcpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
> > diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> > index 29f491082926..8d229a11bb6b 100644
> > --- a/drivers/input/serio/serio.c
> > +++ b/drivers/input/serio/serio.c
> > @@ -694,6 +694,11 @@ EXPORT_SYMBOL(serio_reconnect);
> > */
> > void __serio_register_port(struct serio *serio, struct module *owner)
> > {
> > + if (!serio->write) {
> > + pr_err("%s: refusing to register %s without write method\n",
> > + __func__, serio->name);
> > + return;
> > + }
> > serio_init_port(serio);
> > serio_queue_event(serio, owner, SERIO_REGISTER_PORT);
> > }
> > diff --git a/include/linux/serio.h b/include/linux/serio.h
> > index 6c27d413da92..075f1b8d76fa 100644
> > --- a/include/linux/serio.h
> > +++ b/include/linux/serio.h
> > @@ -121,10 +121,7 @@ void serio_unregister_driver(struct serio_driver *drv);
> >
> > static inline int serio_write(struct serio *serio, unsigned char data)
> > {
> > - if (serio->write)
> > - return serio->write(serio, data);
> > - else
> > - return -1;
> > + return serio->write(serio, data);
> > }
> >
> > static inline void serio_drv_write_wakeup(struct serio *serio)
> > --
> > 2.31.0.rc2.261.g7f71774620-goog
> >
> >
> > --
> > Dmitry
>
> This patch as commit 81c7c0a350bf ("Input: serio - make write method
> mandatory") in -next breaks input for my Hyper-V VM, which prevents me
> from logging in. I attempted to do something like the following (-1 or
> -EINVAL) which should be equivalent but it does not resolve the issue.
>
> Cheers,
> Nathan
>
> diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> index 1a7b72a9016d..d3eee2d4c327 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -311,6 +311,11 @@ static void hv_kbd_stop(struct serio *serio)
> spin_unlock_irqrestore(&kbd_dev->lock, flags);
> }
>
> +static int hv_kbd_write(struct serio *serio, u8 data)
> +{
> + return -1;
> +}
> +
> static int hv_kbd_probe(struct hv_device *hv_dev,
> const struct hv_vmbus_device_id *dev_id)
> {
> @@ -341,6 +346,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
>
> hv_serio->start = hv_kbd_start;
> hv_serio->stop = hv_kbd_stop;
> + hv_serio->write = hv_kbd_write;
>
> error = vmbus_open(hv_dev->channel,
> KBD_VSC_SEND_RING_BUFFER_SIZE,
I'm seeing the same problem. I've added the code to hyperv-keyboard.c that Nathan
proposed, and that solves the immediate problem in that the "refusing to register"
message no longer occurs.
But there's now a different problem in that this error is output whenever a key
is typed on the Hyper-V synthetic keyboard:
[ 11.576716] atkbd serio0: keyboard reset failed on d34b2567-b9b6-42b9-8778-0a4ec0b
The Hyper-V keyboard driver depends on the AT Keyboard driver, and there's code in
atkbd.c that checks for the existence of the serio->write function. I haven't debugged all
the details, but apparently hyperv-keyboard.c depends on atkbd.c finding that function
as NULL in order to work properly. See atkbd_connect(). These messages are output
during boot when the two drivers are working properly together:
[ 2.672693] hv_vmbus: registering driver hyperv_keyboard
[ 2.700587] input: AT Translated Set 2 keyboard as /devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0004:00/VMBUS:00/d34b2567-b9b6-42b9-8778-0a4ec0b955bf/serio0/input/input1
I'm not seeing the second message when running the latest linux-next.
Michael
Powered by blists - more mailing lists