Re: [PATCH 2/3] iio: hid-sensor-humidity: use u32 instead of unsigned int

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Directly dereferencing the `raw_data` pointer as an `s32` can cause unaligned access faults on architectures that strictly enforce alignment.
- [High] The driver unconditionally dereferences `raw_data` as a 32-bit integer, ignoring the `raw_len` parameter, leading to potential out-of-bounds memory reads and corrupted sensor data.
- [High] The `humidity_callbacks` structure is statically allocated but its `pdev` field is modified during `probe`. If multiple humidity sensors are probed, the last probed device overwrites `pdev`, causing callbacks for all instances to be routed to the last bound device's state.
--

commit 90e5b3ef32381e14224dfd5cf7711e2aa3c588a2
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: hid-sensor-humidity: use u32 instead of unsigned int

This commit updates the usage_id parameter type from unsigned int to u32
in callback functions to match the expected API. It is intended to improve
code clarity with no functional changes.

> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index be2338d5f4070..c376a247b1372 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c

[ ... ]

> @@ -141,7 +141,7 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
>  
>  /* Capture samples in local storage */
>  static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
> -				unsigned int usage_id, size_t raw_len,
> +				u32 usage_id, size_t raw_len,
>  				char *raw_data, void *pdev)
>  {

[Severity: High]
This is a pre-existing issue, but does casting the raw_data pointer directly to
an s32 pointer cause unaligned access faults on architectures that strictly
enforce alignment?

drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
	switch (usage_id) {
	case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
		humid_st->scan.humidity_data = *(s32 *)raw_data;
...
}

The raw_data pointer points directly into the packed HID report byte stream,
which may be unaligned.

Could casting it directly to an s32 pointer and dereferencing it trigger
alignment faults on architectures that do not support hardware unaligned
accesses?

[Severity: High]
This is also a pre-existing issue, but does this function unconditionally read 4
bytes from raw_data, ignoring the raw_len parameter?

drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
	switch (usage_id) {
	case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
		humid_st->scan.humidity_data = *(s32 *)raw_data;
...
}

By indiscriminately reading 4 bytes, the driver might access out-of-bounds
memory when raw_len is smaller than 4.

Could this trigger memory access faults near page boundaries and leak kernel
heap data to userspace if the upper bits contain adjacent memory garbage?

>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> @@ -161,7 +161,7 @@ static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
>  static int humidity_parse_report(struct platform_device *pdev,
>  				struct hid_sensor_hub_device *hsdev,
>  				struct iio_chan_spec *channels,
> -				unsigned int usage_id,
> +				u32 usage_id,
>  				struct hid_humidity_state *st)
>  {
>  	int ret;

[Severity: High]
This is a pre-existing issue, but does the shared static humidity_callbacks
structure get overwritten during probe?

drivers/iio/humidity/hid-sensor-humidity.c:
static struct hid_sensor_hub_callbacks humidity_callbacks = {
	.send_event = &humidity_proc_event,
	.capture_sample = &humidity_capture_sample,
};

drivers/iio/humidity/hid-sensor-humidity.c:hid_humidity_probe() {
...
	humidity_callbacks.pdev = pdev;
...
}

If multiple humidity sensors are probed, the last probed device overwrites pdev.

When sensor_hub_raw_event() receives data, it invokes callback->capture_sample
with the overwritten pdev.

Could this cause platform_get_drvdata(pdev) to return the wrong IIO device,
corrupting its state and dropping data for all previously bound sensors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com?part=2