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 |
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"); >
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 --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");
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(-)