Re: [PATCH] HID: usbhid: skip interrupt IN polling for devices with no input reports

[Denis Benato <benato.denis96@gmail.com>]

On 6/5/26 14:02, Antheas Kapenekakis wrote:
> On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@ghoul.dev> wrote:
>> usbhid starts polling a device's interrupt IN endpoint on open
>> (usbhid_open() -> hid_start_in()). If the report descriptor declares no
>> input reports there is nothing to read there, so the poll is useless,
>> and on some composite devices it is also harmful.
> If it did have input reports, would starting the polling still cause
> issues? Because if it would, the issue is in the polling itself.
So far we haven't found an asus device that has more than one interface
that supports reading data out of if.
> Given the creativity of manufacturers when implementing hid protocols,
> I find it certain that they do use the in endpoint even without input
> reports. E.g., for feature reports. This could cause regressions.
While I mostly agree with this it is also true that the general direction
for the kernel (especially lately) has been to not do out-of-spec things
at least by default.

If things really regress it's expected to do so only an very few specific
devices with a buggy firmware, and we can think of something different
for those (hopefully very few ones).

Perhaps someone concerned with security might be interested in what
we have because it doesn't look very normal.

Note that below I have written a few ideas that maybe are worth
looking into.
>> The ASUS ROG N-Key keyboards expose a second, input-less interface used
>> only for RGB control via feature reports. Opening its hidraw node (any
>> hidraw reader does, including SDL/Steam Input or a plain cat) starts the
> cating a hidraw causing issues would be expected, so let's focus on the former.
Simply opening an hidraw should not trigger a delayed disconnect of that device,
I don't know why you would expect this to happen nor why you would
consider it acceptable. It's a bug.

Focusing on userspace software exposing the bug is not a realistic option
because over the time we found a good chunk of software doing that:
- logitech control software (forgot the name)
- open razer software
- sdl
- asusctl (obviously it opens the device albeit in the future I will change this)

and likely more given the fact not all software was identified. 
> Asusctl has a bug where if you add the quirk that separates the event
> nodes per hid, this bug is reproduced as well. I chucked it to
> complicated threading getting out of control. It is the reason we
> skipped that patch that was in my series.
I found and solved the bug already. Regardless the issue remains:
Even with no asusctl at all, if a user has one logitech mouse
(and its control software) and a razer keyboard (and its control software)
the asus N-Key device will start an endless disconnect-reconnect loop.

Any combination of two or more of those tools will trigger the issue
on some devices (weirdly enough not every model is affected):
this is not good.
> Now, you say SDL/Steam do a spurious read as well, can you identify
> the codepath so we can look into it? What devices are affected? The
> early return fixes a warning on the Z13, but it also feeds through the
> universal lamp interface on the new Xbox Allies. Is this a bug on
> those devices or keyboards? If yes, it could be caused by userspace
> hanging on that node
Sure, and I agree with you that fixing all userspace tools is desirable
but it's also unfeasible to fix them all, if we managed to do that
there will be years before everyone receives a fixed version of every
affected software and even then a core issue would remain:
linux tries to poll something it can't have anything out from.

I am much more oriented on the fact that kernel shouldn't
be doing weird things (at least not by default) so this has to
somehow be stopped regardless of how well userspace behaves.

If you have better ideas on how to fix the kernel we would
like to hear those as well.

Best regards,
Denis
> Antheas
>
>> pointless IN poll and keypress reports on the keyboard interface get
>> dropped for as long as the node stays open: a lost key-down drops a
>> letter, a lost key-up leaves the key stuck. usbmon shows the dropped
>> reports never reach the URB layer.
>>
>> The useless poll itself is long-standing; commit 4ac74ea68f64 ("HID:
>> asus: early return for ROG devices") is what exposes it on these
>> devices by keeping the input-less interface alive instead of ejecting
>> it, so its hidraw node can be opened and the poll started.
>>
>> Skip the poll in usbhid_open() when the device has no input reports.
>> Feature reports and hidraw output keep working over the control and OUT
>> endpoints, so the interface is otherwise unaffected.
I will write my review here to avoid forking the discussion:

I agree with the general idea but perhaps we can avoid
some hid devices to ever get HID_QUIRK_ALWAYS_POLL
and that might be enough to skip the problematic code?

Maybe there is value in doing this with a quirk flag in hid-asus.c
affecting the least amount of devices?

Or maybe just prevent devices with no data possibly coming out
to ever get HID_QUIRK_ALWAYS_POLL?

For how to best do this we will need to hear what Jiri and
Benjamin have to say but if they think the proposed solution
is the correct solution:

Reviewed-by: Denis Benato <denis.benato@linux.dev>
>> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
>> Tested-by: Kerim Kabirov <the.privat33r+linux@pm.me>
>> Tested-by: GameBurrow <gameburrow@pm.me>
>> Signed-off-by: Ahmed Yaseen <yaseen@ghoul.dev>
>> ---
>>  drivers/hid/usbhid/hid-core.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>> index 96b0181cf819..90a8b34d9305 100644
>> --- a/drivers/hid/usbhid/hid-core.c
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -688,7 +688,8 @@ static int usbhid_open(struct hid_device *hid)
>>
>>         set_bit(HID_OPENED, &usbhid->iofl);
>>
>> -       if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
>> +       if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) ||
>> +           list_empty(&hid->report_enum[HID_INPUT_REPORT].report_list)) {
>>                 res = 0;
>>                 goto Done;
>>         }
>> --
>> 2.54.0
>>
>>
>>