Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()

[Bhargav Joshi]

On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Sun,  1 Mar 2026 00:43:59 +0530
> Bhargav Joshi <rougueprince47@xxxxxxxxx> wrote:
>
> > Currently, calling iio_device_register() before
> > sensor_hub_register_callback() may create a race condition where the
> > device is exposed to userspace before callbacks are wired.
>
> We needs some more here on 'why' this is a problem.
> Whilst this is an unusual arrangement, I couldn't immediately find
> anything that was actually broken.  It's possible data will turn up
> after we tear down the callbacks and before the userspace interfaces
> are removed, but I think that just results in dropping data. Given it's
> in a race anyway I don't think we care about that. The callbacks
> don't seem to be involved in device configuration which can go on whether
> or not the callbacks are registered.  Maybe there is something that
> won't get acknowledged if they aren't in place in time?
>
> For a fix like this we'd normally want to see a clear flow that
> leads to a bug.
>
> Also a fixes tag so we know how far to backport.
>

Hi Jonathan,

I originally submitted the patch for the driver because calling
iio_device_register() before the callbacks are wired up violates
standard IIO LIFO ordering. I assumed this created a dangerous race
condition with userspace. However, as you correctly pointed out, the
HID sensor hub safely drops those early events without crashing, even
if it is unusual behaviour still won't have a hard crash.

My underlying goal was simply a structural cleanup to align the
probe() and remove() sequences with standard IIO architecture.

Would you like me to send a v2 of this patch with a revised commit
message classifying it purely as a structural cleanup (and without a
Fixes tag), or is the current driver behavior acceptable enough that I
should just drop this patch entirely?

Thanks,
Bhargav

> >
> > Move iio_device_register() to the end of the probe() function to prevent
> > race condition.
> >
> > Consequently, update the error handling path in probe() and in remove()
> > ensuring that iio_device_unregister() is called first to cut off
> > userspace access before the hardware callbacks are removed.
> >
> > Signed-off-by: Bhargav Joshi <rougueprince47@xxxxxxxxx>
> > ---
> >  drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > index c43990c518f7..8e3628cd8529 100644
> > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> >               return ret;
> >       }
> >
> > -     ret = iio_device_register(indio_dev);
> > -     if (ret) {
> > -             dev_err(&pdev->dev, "device register failed\n");
> > -             goto error_remove_trigger;
> > -     }
> > -
> >       gyro_state->callbacks.send_event = gyro_3d_proc_event;
> >       gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
> >       gyro_state->callbacks.pdev = pdev;
> > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> >                                       &gyro_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_GYRO_3D);
> >  error_remove_trigger:
> >       hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> >       return ret;
> > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> >       struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> >       struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> >
> > -     sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> >       iio_device_unregister(indio_dev);
> > +     sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> >       hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> >  }
> >
>