diff mbox series

iio: hid-sensor-attributes: validate sensitivity attributes

Message ID 20250109040006.1273797-1-acelan.kao@canonical.com (mailing list archive)
State Superseded
Headers show
Series iio: hid-sensor-attributes: validate sensitivity attributes | expand

Commit Message

Chia-Lin Kao (AceLan) Jan. 9, 2025, 4 a.m. UTC
An invalid sensor device was observed which provided valid index and
report_ids for poll, report_state and power_state attributes, but had
invalid report_latency, sensitivity, and timestamp attributes. This would
cause the system to hang when using iio_info to access attributes, as
runtime PM tried to wake up an unresponsive sensor.

[    2.594565] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
[    2.594573] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:1, 2:1, 3:1 ffffffff:ffffffff ffffffff:ffffffff
[    2.595485] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
[    2.595492] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:11, 3:11, 1:11 ffffffff:ffffffff ffffffff:ffffffff

Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 .../hid-sensors/hid-sensor-attributes.c       | 23 +++++++++++--------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Jonathan Cameron Jan. 12, 2025, 2:59 p.m. UTC | #1
On Thu,  9 Jan 2025 12:00:06 +0800
"Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com> wrote:

> An invalid sensor device was observed which provided valid index and
> report_ids for poll, report_state and power_state attributes, but had
> invalid report_latency, sensitivity, and timestamp attributes. This would
> cause the system to hang when using iio_info to access attributes, as
> runtime PM tried to wake up an unresponsive sensor.
> 
> [    2.594565] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
> [    2.594573] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:1, 2:1, 3:1 ffffffff:ffffffff ffffffff:ffffffff
> [    2.595485] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
> [    2.595492] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:11, 3:11, 1:11 ffffffff:ffffffff ffffffff:ffffffff
> 
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
If you can come up with an appropriate fixes tag that would be great
as will help us figure out how far this might be backported.

Also, can we add any info on what device this was seen on?
+CC Jiri and Srinivas who are the other listed maintainers of this driver.

Thanks,

Jonathan

> ---
>  .../hid-sensors/hid-sensor-attributes.c       | 23 +++++++++++--------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index ad1882f608c0..b7ffd97e6c56 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> @@ -564,8 +564,21 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
>  	} else
>  		st->timestamp_ns_scale = 1000000000;
>  
> +	ret = 0;
> +	if (st->sensitivity.index < 0 || st->sensitivity_rel.index < 0) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
>  	hid_sensor_get_report_latency_info(hsdev, usage_id, st);
>  
> +	ret = sensor_hub_get_feature(hsdev,
> +				st->power_state.report_id,
> +				st->power_state.index, sizeof(value), &value);
> +	if (value < 0)
> +		ret = -EINVAL;
> +
> +out:
>  	hid_dbg(hsdev->hdev, "common attributes: %x:%x, %x:%x, %x:%x %x:%x %x:%x\n",
>  		st->poll.index, st->poll.report_id,
>  		st->report_state.index, st->report_state.report_id,
> @@ -573,15 +586,7 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
>  		st->sensitivity.index, st->sensitivity.report_id,
>  		timestamp.index, timestamp.report_id);
>  
> -	ret = sensor_hub_get_feature(hsdev,
> -				st->power_state.report_id,
> -				st->power_state.index, sizeof(value), &value);
> -	if (ret < 0)
> -		return ret;
> -	if (value < 0)
> -		return -EINVAL;
> -
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL_NS(hid_sensor_parse_common_attributes, "IIO_HID");
>
Chia-Lin Kao (AceLan) Jan. 13, 2025, 5:16 a.m. UTC | #2
On Sun, Jan 12, 2025 at 02:59:38PM +0000, Jonathan Cameron wrote:
> On Thu,  9 Jan 2025 12:00:06 +0800
> "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com> wrote:
> 
> > An invalid sensor device was observed which provided valid index and
> > report_ids for poll, report_state and power_state attributes, but had
> > invalid report_latency, sensitivity, and timestamp attributes. This would
> > cause the system to hang when using iio_info to access attributes, as
> > runtime PM tried to wake up an unresponsive sensor.
> > 
> > [    2.594565] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
> > [    2.594573] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:1, 2:1, 3:1 ffffffff:ffffffff ffffffff:ffffffff
> > [    2.595485] [453] hid-sensor-hub 0003:0408:5473.0003: Report latency attributes: ffffffff:ffffffff
> > [    2.595492] [453] hid-sensor-hub 0003:0408:5473.0003: common attributes: 5:11, 3:11, 1:11 ffffffff:ffffffff ffffffff:ffffffff
> > 
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> If you can come up with an appropriate fixes tag that would be great
> as will help us figure out how far this might be backported.
Sure, I'll add the following line in v2
Fixes: bba6d9e47f3e ("iio: hid-sensor-attributes: Fix sensor property setting failure.")

> 
> Also, can we add any info on what device this was seen on?
This invalid sensor is found on this USB camera[0408:5473]
I'll also add this to the v2 commit message.

T:  Bus=03 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  2 Spd=480  MxCh= 0
D:  Ver= 2.01 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0408 ProdID=5473 Rev=00.07
S:  Manufacturer=Quanta
S:  Product=HP 5MP Camera
S:  SerialNumber=01.00.00
C:  #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=01 Driver=uvcvideo
E:  Ad=87(I) Atr=03(Int.) MxPS=  16 Ivl=16ms
I:  If#= 1 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=01 Driver=uvcvideo
I:  If#= 2 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=01 Driver=uvcvideo
E:  Ad=85(I) Atr=03(Int.) MxPS=  16 Ivl=16ms
I:  If#= 3 Alt= 0 #EPs= 0 Cls=0e(video) Sub=02 Prot=01 Driver=uvcvideo
I:  If#= 4 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=84(I) Atr=03(Int.) MxPS=  64 Ivl=16ms
I:  If#= 5 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=01 Driver=(none)

> +CC Jiri and Srinivas who are the other listed maintainers of this driver.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  .../hid-sensors/hid-sensor-attributes.c       | 23 +++++++++++--------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > index ad1882f608c0..b7ffd97e6c56 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > @@ -564,8 +564,21 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
> >  	} else
> >  		st->timestamp_ns_scale = 1000000000;
> >  
> > +	ret = 0;
> > +	if (st->sensitivity.index < 0 || st->sensitivity_rel.index < 0) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> >  	hid_sensor_get_report_latency_info(hsdev, usage_id, st);
> >  
> > +	ret = sensor_hub_get_feature(hsdev,
> > +				st->power_state.report_id,
> > +				st->power_state.index, sizeof(value), &value);
> > +	if (value < 0)
> > +		ret = -EINVAL;
> > +
> > +out:
> >  	hid_dbg(hsdev->hdev, "common attributes: %x:%x, %x:%x, %x:%x %x:%x %x:%x\n",
> >  		st->poll.index, st->poll.report_id,
> >  		st->report_state.index, st->report_state.report_id,
> > @@ -573,15 +586,7 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
> >  		st->sensitivity.index, st->sensitivity.report_id,
> >  		timestamp.index, timestamp.report_id);
> >  
> > -	ret = sensor_hub_get_feature(hsdev,
> > -				st->power_state.report_id,
> > -				st->power_state.index, sizeof(value), &value);
> > -	if (ret < 0)
> > -		return ret;
> > -	if (value < 0)
> > -		return -EINVAL;
> > -
> > -	return 0;
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL_NS(hid_sensor_parse_common_attributes, "IIO_HID");
> >  
>
diff mbox series

Patch

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
index ad1882f608c0..b7ffd97e6c56 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
@@ -564,8 +564,21 @@  int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
 	} else
 		st->timestamp_ns_scale = 1000000000;
 
+	ret = 0;
+	if (st->sensitivity.index < 0 || st->sensitivity_rel.index < 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	hid_sensor_get_report_latency_info(hsdev, usage_id, st);
 
+	ret = sensor_hub_get_feature(hsdev,
+				st->power_state.report_id,
+				st->power_state.index, sizeof(value), &value);
+	if (value < 0)
+		ret = -EINVAL;
+
+out:
 	hid_dbg(hsdev->hdev, "common attributes: %x:%x, %x:%x, %x:%x %x:%x %x:%x\n",
 		st->poll.index, st->poll.report_id,
 		st->report_state.index, st->report_state.report_id,
@@ -573,15 +586,7 @@  int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
 		st->sensitivity.index, st->sensitivity.report_id,
 		timestamp.index, timestamp.report_id);
 
-	ret = sensor_hub_get_feature(hsdev,
-				st->power_state.report_id,
-				st->power_state.index, sizeof(value), &value);
-	if (ret < 0)
-		return ret;
-	if (value < 0)
-		return -EINVAL;
-
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_NS(hid_sensor_parse_common_attributes, "IIO_HID");