linux-kernel - Re: [PATCH] hid: usbhid: Enable remote wake-up based on device configuration

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPwe5ROTfQVQ2fF3ab05E51X+_5zFpSNK-qrEh-ev-WWBzY+DA@mail.gmail.com>
Date: Mon, 15 Jul 2024 23:36:57 +0800
From: ryan zhou <ryanzhou54@...il.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Alan Stern <stern@...land.harvard.edu>, jikos@...nel.org, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hid: usbhid: Enable remote wake-up based on device configuration

On Thu, Jul 11, 2024 at 3:41 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Wed, Jul 10, 2024 at 09:47:39PM -0400, Alan Stern wrote:
> > On Thu, Jul 11, 2024 at 07:16:06AM +0800, ryan wrote:
> > > According to the USB protocol, the host should automatically
> > > adapt the remote wake-up function based on the configuration
> > > descriptor reported by the device, rather than only the default
> > > keyboard support. Therefore, it's necessary to support other hid
> > > devices, such as digital headsets,mice,etc.
> >
> > It's true that the host shouldn't try to enable remote wakeup if the
> > configuration descriptor shows that the device doesn't support it.
> >
> > However, it's not true that the host should try to enable remote wakeup
> > for devices other than keyboards with boot-protocol support.  History
> > has shown that quite a few HID devices don't handle remote wakeup
> > properly; the decision about whether to enable it should be left to the
> > user.
>
> I agree, this patch isn't acceptable.  Ryan, why do you want this
> applied?  What userspace control is missing to allow you to do this
> today on your systems with no kernel changes for devices that you know
> will work properly?
>
> thanks,
>
> greg k-h

Many thanks to Greg KH and Alan Stern for reviewing the patch and
replying to me.
I'd like to start by asking Greg KH's question.

A1:This patch is expected to be applied to the USB digital headset,
mouse, and keyboard,
and we expect to wake up the system by operating them when the system
has suspended.

A2:I've verified that user-space control does the trick, but
Personally speaking, it's not a good solution.
For each device plugged into the host, the user space needs to check whether
it is one of the three and to enable wakeup.It may be better to enable
wakeup when loading
a HID class drivers, from my perspective. Could you please give me
some advice if possible.

I have spent some time studying your responses, and learned a lot. I
absolutely agree with many
of your points, but still have some doubts.

Q1 for Alan Stern: Boot device includes a boot mouse and boot keyboard,
why the patch(3d61510f4ecac) only enables boot keyboard by default,
and in addation boot
protocol is used in BIOS,why is it used as a wakeup judgment condition
in the OS?

Q2: for Alan Stern:  As you comment 'History has shown that quite a
few HID devices don't
handle remote wakeup properly'  I consulted the USB20 Spec in Chapter
9.2.5.2 and it has
this description:'If a device supports remote wakeup, it must also
allow the capability to be
enabled and disabled using the standard USB request'  So these devices
that you're talking about
are not compliant with the USB20 protocol specification to my mind. If
so, shouldn't we
support these non-standard devices.

Thanks

ryan