On Wed, Jun 19, 2019 at 10:47:34AM +0200, Christian Schneider wrote:
> Hi,
> unfrotunately I found a problem with my patch. It didn't appear on the
> devices, where I tested, only when distributing the patch to more devices,
> it occurs sometimes.
>
> There is a possible race condition. In gpio_fan_probe, the IRQ handler for
> the fan alarm is registered before the hwmon_dev is initialised.
> So it is possible, that the handler executes, while fandata->hwmon_dev is
> still unitialised and the handler runs in an invalid pointer dereference.
>
> The obvious fix, moving the
> /* Configure alarm GPIO if available. */
> if (fan_data->alarm_gpio) {
> err = fan_alarm_init(fan_data);
> if (err)
> return err;
> }
> block after
>
> /* Make this driver part of hwmon class. */
> fan_data->hwmon_dev =
> devm_hwmon_device_register_with_groups(dev,
> "gpio_fan", fan_data,
> gpio_fan_groups);
>
> doesn't work. Sysfs doesn't have a fan1_alarm attribute anymore then.
>
> It is not yet clear to me, why creation of the attribute fails in this case.
> I need to investigate further.
> In the meanwhile, please revert my patch.
>
> I'm sorry for the disturbance I created....
>
No worries. Thanks for noticing and for reporting the problem. I dropped
the patch from the queue for now.
Guenter
> BR, Christian
>
>
> Am 17.06.2019 um 18:04 schrieb Guenter Roeck:
> >On Mon, Jun 17, 2019 at 11:33:43AM +0200, cschneider@xxxxxxxxxxxxx wrote:
> >>From: Christian Schneider <christian@xxxxxxxx>
> >>
> >>sysfs_notify and kobject_uevent are passed the wrong kobject.
> >>that why notifications can't be received and uevents have the wrong path.
> >>this patch fixes this.
> >>
> >>Signed-off-by: Christian Schneider <cschneider@xxxxxxxxxxxxx>
> >
> >Applied.
> >
> >In the future, please version your patches, add a change log,
> >and reflect the subsystem and driver in the subject line.
> >
> >Thanks,
> >Guenter
> >
> >>---
> >> drivers/hwmon/gpio-fan.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> >>index 84753680a4e8..76377791ff0e 100644
> >>--- a/drivers/hwmon/gpio-fan.c
> >>+++ b/drivers/hwmon/gpio-fan.c
> >>@@ -54,8 +54,8 @@ static void fan_alarm_notify(struct work_struct *ws)
> >> struct gpio_fan_data *fan_data =
> >> container_of(ws, struct gpio_fan_data, alarm_work);
> >>- sysfs_notify(&fan_data->dev->kobj, NULL, "fan1_alarm");
> >>- kobject_uevent(&fan_data->dev->kobj, KOBJ_CHANGE);
> >>+ sysfs_notify(&fan_data->hwmon_dev->kobj, NULL, "fan1_alarm");
> >>+ kobject_uevent(&fan_data->hwmon_dev->kobj, KOBJ_CHANGE);
> >> }
> >> static irqreturn_t fan_alarm_irq_handler(int irq, void *dev_id)
