Re: [PATCH v4 2/6] iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The `struct iio_scan_type` is incorrectly initialized with a non-existent `.format` member, causing a build failure.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a teardown race condition exists when the driver is unbound, leading to a potential Use-After-Free of `gyro_state` inside the HID raw event handler.
--

commit fa4886d3bbba316658546fb6162186593b204b9c
Author: Natália Salvino André <natalia.andre@ime.usp.br>

iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization

This commit replaces the local gyro_3d_adjust_channel_bit_mask() function
with a compound literal for scan_type initialization to improve code
readability. It also cleans up the channel initialization loop by iterating
directly over the channel scan indices.

> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index c43990c518f77..ee7bde8620b41 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
[ ... ]
> @@ -248,19 +238,20 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
>  				struct gyro_3d_state *st)
>  {
>  	int ret;
> -	int i;
>  
> -	for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> +	for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
>  		ret = sensor_hub_input_get_attribute_info(hsdev,
>  				HID_INPUT_REPORT,
>  				usage_id,
> -				HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + i,
> -				&st->gyro[CHANNEL_SCAN_INDEX_X + i]);
> +				HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
> +				&st->gyro[ch]);
>  		if (ret < 0)
>  			break;
> -		gyro_3d_adjust_channel_bit_mask(channels,
> -				CHANNEL_SCAN_INDEX_X + i,
> -				st->gyro[CHANNEL_SCAN_INDEX_X + i].size);
> +		channels[ch].scan_type = (struct iio_scan_type) {
> +			.format = 's',

[Severity: Low]
Does this initialization cause a build failure?

Looking at struct iio_scan_type, it designates the field for signedness as sign,
not format. The original code correctly assigned 's' to
channels[channel].scan_type.sign. Using format here will likely cause a compiler
error about having no member named format.

> +			.realbits = BYTES_TO_BITS(st->gyro[ch].size),
> +			.storagebits = BITS_PER_TYPE(u32),
> +		};
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but could a teardown race condition
exist when the driver is unbound, leading to a potential use-after-free of
gyro_state inside the HID raw event handler?

When the driver is manually unbound, hid_gyro_3d_remove() calls
sensor_hub_remove_callback(). This core function unregisters the callback list
entry but does not flush or synchronize with concurrently running
sensor_hub_raw_event() asynchronous dispatches.

Consequently, remove completes and devres proceeds to free indio_dev, which
embeds gyro_state and the callback structure. A concurrent
sensor_hub_raw_event() that fetched the callback pointer prior to removal (after
dropping dyn_callback_lock) could then execute callback->capture_sample() and
callback->send_event() on freed memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=2