* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 15:43 [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible Kittisak Boonmapa
@ 2026-07-01 15:49 ` Andy Shevchenko
2026-07-01 16:22 ` Joshua Crofts
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-07-01 15:49 UTC (permalink / raw)
To: Kittisak Boonmapa
Cc: jic23, linux-iio, linux-kernel, dlechner, nuno.sa, andy,
srinivas.pandruvada, sakari.ailus, joshua.crofts1
On Wed, Jul 01, 2026 at 10:43:19PM +0700, Kittisak Boonmapa 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.
Makes sense.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 15:43 [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible Kittisak Boonmapa
2026-07-01 15:49 ` Andy Shevchenko
@ 2026-07-01 16:22 ` Joshua Crofts
2026-07-01 17:42 ` srinivas pandruvada
2026-07-01 17:47 ` Jonathan Cameron
3 siblings, 0 replies; 7+ messages in thread
From: Joshua Crofts @ 2026-07-01 16:22 UTC (permalink / raw)
To: Kittisak Boonmapa
Cc: jic23, linux-iio, linux-kernel, dlechner, nuno.sa, andy,
srinivas.pandruvada, sakari.ailus
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>
>
> ---
This bug was introduced 9 years ago!
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
--
Kind regards
CJD
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 15:43 [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible Kittisak Boonmapa
2026-07-01 15:49 ` Andy Shevchenko
2026-07-01 16:22 ` Joshua Crofts
@ 2026-07-01 17:42 ` srinivas pandruvada
2026-07-01 17:47 ` Jonathan Cameron
3 siblings, 0 replies; 7+ messages in thread
From: srinivas pandruvada @ 2026-07-01 17:42 UTC (permalink / raw)
To: Kittisak Boonmapa, jic23
Cc: linux-iio, linux-kernel, dlechner, nuno.sa, andy, sakari.ailus,
joshua.crofts1
On Wed, 2026-07-01 at 22:43 +0700, Kittisak Boonmapa 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>
>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> 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.
>
> 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.
Although Report interval is an optional field, every hub had it from
the beginning.
But this change is good.
Thanks,
Srinivas
>
> 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;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 15:43 [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible Kittisak Boonmapa
` (2 preceding siblings ...)
2026-07-01 17:42 ` srinivas pandruvada
@ 2026-07-01 17:47 ` Jonathan Cameron
2026-07-01 17:59 ` srinivas pandruvada
3 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-07-01 17:47 UTC (permalink / raw)
To: Kittisak Boonmapa
Cc: linux-iio, linux-kernel, dlechner, nuno.sa, andy,
srinivas.pandruvada, sakari.ailus, joshua.crofts1
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;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 17:47 ` Jonathan Cameron
@ 2026-07-01 17:59 ` srinivas pandruvada
2026-07-01 18:59 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: srinivas pandruvada @ 2026-07-01 17:59 UTC (permalink / raw)
To: Jonathan Cameron, Kittisak Boonmapa
Cc: linux-iio, linux-kernel, dlechner, nuno.sa, andy, sakari.ailus,
joshua.crofts1
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@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.
>
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;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix poll_value sign check before msleep_interruptible
2026-07-01 17:59 ` srinivas pandruvada
@ 2026-07-01 18:59 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-07-01 18:59 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Kittisak Boonmapa, linux-iio, linux-kernel, dlechner, nuno.sa,
andy, sakari.ailus, joshua.crofts1
On Wed, 01 Jul 2026 10:59:00 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> 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@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.
> >
> 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;
^ permalink raw reply [flat|nested] 7+ messages in thread