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

[Jonathan Cameron]

On Wed, 01 Jul 2026 10:59:00 -0700
srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:

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

Ah. Fair enough.  In that case as below, explicitly catch the error
and put a default in place rather than relying on an error code in
a variable that isn't obviously going to contain one.

Jonathan

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