Re: [PATCH 8/9] iio: magnetometer: hid-sensor-magn-3d: Fix race between callback registration and device exposure

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Reordering `iio_device_unregister()` before `sensor_hub_remove_callback()` in the remove path introduces a critical list corruption race.
- [High] Reordering callback registration before IIO device registration introduces a Use-After-Free (UAF) window on the probe error path.

Pre-existing issues:
- [High] Pre-existing Use-After-Free vulnerability because `sensor_hub_remove_callback()` unlinks the callback without synchronizing with currently executing asynchronous HID events.
--

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

iio: magnetometer: hid-sensor-magn-3d: Fix race between callback registration and device exposure

This patch attempts to fix a race condition by reordering the callback
registration and IIO device registration in the probe and remove paths.
It registers the sensor hub callback before exposing the IIO device, and
unregisters the IIO device before removing the callbacks.

> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 23884825eb00f..acae77273bd5e 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
[ ... ]
> @@ -526,13 +520,19 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
>  					&magn_state->callbacks);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		goto error_iio_unreg;
> +		goto error_remove_trigger;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "device register failed\n");
> +		goto error_remove_callback;
>  	}
>  
>  	return ret;
>  
> -error_iio_unreg:
> -	iio_device_unregister(indio_dev);
> +error_remove_callback:
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);

[Severity: High]
Can this sequence cause a use-after-free if a callback is currently
executing?

Since sensor_hub_remove_callback() only unlinks the callback without
synchronizing or waiting for currently executing ones to finish, could
magn_3d_proc_event() still be running?

When hid_magn_3d_probe() returns an error, the devres subsystem will
immediately free indio_dev and magn_state. If a concurrent callback was
triggered right before iio_device_register() failed, it would then
access the newly freed memory.

>  error_remove_trigger:
>  	hid_sensor_remove_trigger(indio_dev, &magn_state->magn_flux_attributes);
>  	return ret;
> @@ -545,8 +545,8 @@ static void hid_magn_3d_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct magn_3d_state *magn_state = iio_priv(indio_dev);
>  
> -	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);
>  	iio_device_unregister(indio_dev);

[Severity: Critical]
Does reordering iio_device_unregister() before removing the callback
expose a list corruption race?

iio_device_unregister() calls iio_disable_all_buffers(), which modifies
the buffer_list using list_del_init() without lock or RCU protection.

Since the sensor callback is still active, incoming hardware events can
continue to fire. If magn_3d_proc_event() calls 
iio_push_to_buffers_with_timestamp() and iterates over buffer_list
locklessly at the same time list_del_init() is executing, could this
cause the next pointer to loop on itself and result in a hard lockup?

> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of
synchronization in sensor_hub_remove_callback() leave a lingering
use-after-free window here?

sensor_hub_remove_callback() removes the callback from the list but
returns without waiting for any currently executing sensor_hub_raw_event()
to finish.

After sensor_hub_remove_callback() completes, hid_magn_3d_remove()
returns and devres will free indio_dev and magn_state. If a callback is
still executing on another CPU, it might access this freed memory.

>  	hid_sensor_remove_trigger(indio_dev, &magn_state->magn_flux_attributes);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=8