Re: [PATCH] hwmon: (sht15) Stop deferred work before releasing its resources

[Guenter Roeck]

On Thu, Oct 15, 2026 at 11:07:49PM -0400, Myeonghun Pak wrote:
> A measurement can return while read_work still accesses GPIOs or
> re-enables the IRQ. Regulator notifications can likewise leave
> update_supply_work using the regulator after it is released.
> 
> On removal and the applicable probe error paths, disable and drain
> read_work before freeing the IRQ. Unregister the regulator notifier
> before canceling update_supply_work.
> 
> This issue was identified as part of our ongoing research while
> reviewing kernel code.
> 
> Fixes: 251eb40f5ccd ("hwmon: sht15 humidity sensor driver")
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> ---
> Compile-tested with GCC, W=1 (x86_64). No runtime testing.
> 
>  drivers/hwmon/sht15.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/sht15.c b/drivers/hwmon/sht15.c
> index 7bb2105..6c5b2a1 100644
> --- a/drivers/hwmon/sht15.c
> +++ b/drivers/hwmon/sht15.c
> @@ -31,6 +31,7 @@
>  #include <linux/bitrev.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/of.h>
> +#include <linux/workqueue.h>
>  
>  /* Commands */
>  #define SHT15_MEASURE_TEMP		0x03
> @@ -986,15 +987,15 @@ static int sht15_probe(struct platform_device *pdev)
>  	disable_irq_nosync(gpiod_to_irq(data->data));
>  	ret = sht15_connection_reset(data);
>  	if (ret)
> -		goto err_release_reg;
> +		goto err_release_irq;
>  	ret = sht15_soft_reset(data);
>  	if (ret)
> -		goto err_release_reg;
> +		goto err_release_irq;
>  
>  	ret = sysfs_create_group(&pdev->dev.kobj, &sht15_attr_group);
>  	if (ret) {
>  		dev_err(&pdev->dev, "sysfs create failed\n");
> -		goto err_release_reg;
> +		goto err_release_irq;
>  	}
>  
>  	data->hwmon_dev = hwmon_device_register(data->dev);
> @@ -1007,9 +1008,13 @@ static int sht15_probe(struct platform_device *pdev)
>  
>  err_release_sysfs_group:
>  	sysfs_remove_group(&pdev->dev.kobj, &sht15_attr_group);
> +err_release_irq:
> +	disable_work_sync(&data->read_work);
> +	devm_free_irq(&pdev->dev, gpiod_to_irq(data->data), data);

With this here and in the remove function, what is the point of using
devm_request_irq() ?

Guenter

>  err_release_reg:
>  	if (!IS_ERR(data->reg)) {
>  		regulator_unregister_notifier(data->reg, &data->nb);
> +		cancel_work_sync(&data->update_supply_work);
>  		regulator_disable(data->reg);
>  	}
>  	return ret;
> @@ -1023,12 +1028,17 @@ static void sht15_remove(struct platform_device *pdev)
>  	hwmon_device_unregister(data->hwmon_dev);
>  	sysfs_remove_group(&pdev->dev.kobj, &sht15_attr_group);
>  
> +	/* The read work can re-enable the IRQ, so stop it before freeing IRQ. */
> +	disable_work_sync(&data->read_work);
> +	devm_free_irq(&pdev->dev, gpiod_to_irq(data->data), data);
> +
>  	ret = sht15_soft_reset(data);
>  	if (ret)
>  		dev_err(&pdev->dev, "Failed to reset device (%pe)\n", ERR_PTR(ret));
>  
>  	if (!IS_ERR(data->reg)) {
>  		regulator_unregister_notifier(data->reg, &data->nb);
> +		cancel_work_sync(&data->update_supply_work);
>  		regulator_disable(data->reg);
>  	}
>  }