[PATCH] iio: hid-sensor-attributes: validate sensitivity attributes

Chia-Lin Kao (AceLan) posted 1 patch 11 months, 2 weeks ago
There is a newer version of this series
.../hid-sensors/hid-sensor-attributes.c       | 23 +++++++++++--------
1 file changed, 14 insertions(+), 9 deletions(-)
[PATCH] iio: hid-sensor-attributes: validate sensitivity attributes
Posted by Chia-Lin Kao (AceLan) 11 months, 2 weeks ago
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(-)

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");
 
-- 
2.43.0
Re: [PATCH] iio: hid-sensor-attributes: validate sensitivity attributes
Posted by Jonathan Cameron 11 months, 1 week ago
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");
>
Re: [PATCH] iio: hid-sensor-attributes: validate sensitivity attributes
Posted by Chia-Lin Kao (AceLan) 11 months, 1 week ago
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");
> >  
>