Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible

[srinivas pandruvada]

On Wed, 2026-07-01 at 18:47 +0100, Jonathan Cameron wrote:
> On Wed,  1 Jul 2026 22:43:19 +0700
> Kittisak Boonmapa <goorock.goopop@xxxxxxxxx> wrote:
> 
> > hid_sensor_read_poll_value() returns -EINVAL when the HID
> > descriptor
> > does not contain a Report Interval feature field.
> > 
> > _hid_sensor_power_state() currently treats any non-zero value as a
> > valid delay and passes it to msleep_interruptible(). Since
> > msleep_interruptible() takes an unsigned int, negative values are
> > converted into very large delays.
> > 
> > Only sleep when poll_value is positive.
> > 
> > Fixes: 5d9854eaea77 ("iio: hid-sensor: Store restore poll and
> > hysteresis on S3")
> > Closes:
> > https://lore.kernel.org/linux-iio/CAPr6G1qLDrgHvCNsVxj7xHxYUKkAkyo87Hq3Lfyoj3RaZ2v4dg@xxxxxxxxxxxxxx/
> > Reported-by: Kittisak Boonmapa <goorock.goopop@xxxxxxxxx>
> > Assisted-by: Anthropic:Claude Sonnet 4.6
> > Signed-off-by: Kittisak Boonmapa <goorock.goopop@xxxxxxxxx>
> > 
> Bug looks to be correct, but fix it by returning much earlier.
> If that function is returning an error something is very wrong and
> we should fail at that point, not just skip a sleep later.
> 
Even if the report interval is not specified, but it has power state,
so returning early will fail to set power state.


Thanks,
Srinivas


> > ---
> > The bug was discovered while implementing a custom USB HID Sensor
> > (Accelerometer 3D, HID Usage 0x200073) on a Seeed XIAO nRF52840
> > Sense
> > for use with the Linux IIO subsystem and iio-sensor-proxy.
> > 
> > The device intentionally omitted the Report Interval feature from
> > its
> > HID descriptor. This causes hid_sensor_read_poll_value() to return
> > -EINVAL, which is then treated as a non-zero delay by
> > _hid_sensor_power_state() and passed directly to
> > msleep_interruptible(). Since msleep_interruptible() takes an
> > unsigned int, the negative value is converted into an unintended
> > sleep of approximately 49.7 days.
> > 
> 
> For this non implemented feature, we need to explicitly handle the
> error and if seen put an appropriate replacement value in place.
> That may well be zero. I'm not sure!  Setting such a 'default'
> value provides a place to add a comment on why we are doing so.
> 
> Jonathan
> 
> > The issue was reproduced consistently on a Steam Deck LCD running
> > Bazzite (Linux 6.17.x), and disappeared completely after adding the
> > Report Interval feature to the HID descriptor, confirming the root
> > cause.
> > 
> > This patch changes the condition to sleep only when poll_value is
> > strictly positive, avoiding unintended delays while preserving the
> > existing behavior for valid poll intervals.
> > 
> >  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > index 417c4ab8c1b2..20099614bb27 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > @@ -143,7 +143,7 @@ static int _hid_sensor_power_state(struct
> > hid_sensor_common *st, bool state)
> >  	sensor_hub_get_feature(st->hsdev, st-
> > >power_state.report_id,
> >  			       st->power_state.index,
> >  			       sizeof(state_val), &state_val);
> > -	if (state && poll_value)
> > +	if (state && poll_value > 0)
> >  		msleep_interruptible(poll_value * 2);
> >  
> >  	return 0;