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

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  1 Jul 2026 22:43:19 +0700
Kittisak Boonmapa <goorock.goopop@gmail.com> 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@mail.gmail.com/
> Reported-by: Kittisak Boonmapa <goorock.goopop@gmail.com>
> Assisted-by: Anthropic:Claude Sonnet 4.6
> Signed-off-by: Kittisak Boonmapa <goorock.goopop@gmail.com>
> 
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.

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