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

[Antheas Kapenekakis]

On Sat, 6 Jun 2026 at 14:42, Denis Benato <benato.denis96@xxxxxxxxx> wrote:
>
>
> On 6/5/26 14:02, Antheas Kapenekakis wrote:
> > On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@xxxxxxxxx> 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

The degradation would be silent.

> 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.

Try to add spaces before and after your responses

> 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.

The kernel is not doing weird things and I also did not ask you to fix
all userspace software. I asked for a reproduction scenario, as it is
not covered in the patch description. Relooking at the patch today, I
also do not understand what it does fully.

It skips enabling input interrupts (but not only that) for devices
that have no input reports. So the kernel behavior will depend on the
feature descriptor moving forward.

And that fixes a hang on the affected devices because enabling
interrupts on an endpoint without periodic input reports blocks a
parallel endpoint that does have input reports?

I would like this fix to target the actual cause that causes the block
but it is not clear to me what that is or what is affected.

Antheas

> 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@xxxxxxxxx>
> >> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
> >> Tested-by: Kerim Kabirov <the.privat33r+linux@xxxxx>
> >> Tested-by: GameBurrow <gameburrow@xxxxx>
> >> Signed-off-by: Ahmed Yaseen <yaseen@xxxxxxxxx>
> >> ---
> >>  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
> >>
> >>
> >>
>