Skip to content

Commit b09a456

Browse files
committed
hwmon: (lm90) Stop work before releasing hwmon device
Sashiko reports: In lm90_probe(), the devm action to cancel the alert_work and report_work (lm90_restore_conf) is registered in lm90_init_client() before devm_hwmon_device_register_with_info() is called. Because devm executes cleanup actions in reverse order during module unbind or probe failure, the hwmon device is unregistered and freed first. If lm90_alert_work() or lm90_report_alarms() runs in the window between the hwmon device being freed and the delayed works being cancelled, lm90_update_alarms() will dereference the freed data->hwmon_dev here. Fix the problem by canceling the workers separately after registering the hwmon device and before registering the interrupt handler. This ensures that the workers are canceled after interrupts are disabled and before the hwmon device is released. Add "shutdown" flag to indicate that device shutdown is in progress to prevent workers from being re-armed. Fixes: f6d0775 ("hwmon: (lm90) Rework alarm/status handling") Reported-by: Sashiko <sashiko-bot@kernel.org> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
1 parent 51f5760 commit b09a456

1 file changed

Lines changed: 20 additions & 4 deletions

File tree

drivers/hwmon/lm90.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,7 @@ struct lm90_data {
736736
struct hwmon_chip_info chip;
737737
struct delayed_work alert_work;
738738
struct work_struct report_work;
739+
bool shutdown; /* true if shutting down */
739740
bool valid; /* true if register values are valid */
740741
bool alarms_valid; /* true if status register values are valid */
741742
unsigned long last_updated; /* in jiffies */
@@ -1154,6 +1155,9 @@ static void lm90_report_alarms(struct work_struct *work)
11541155

11551156
static int lm90_update_alarms_locked(struct lm90_data *data, bool force)
11561157
{
1158+
if (data->shutdown)
1159+
return 0;
1160+
11571161
if (force || !data->alarms_valid ||
11581162
time_after(jiffies, data->alarms_updated + msecs_to_jiffies(data->update_interval))) {
11591163
struct i2c_client *client = data->client;
@@ -2584,15 +2588,23 @@ static void lm90_restore_conf(void *_data)
25842588
struct lm90_data *data = _data;
25852589
struct i2c_client *client = data->client;
25862590

2587-
cancel_delayed_work_sync(&data->alert_work);
2588-
cancel_work_sync(&data->report_work);
2589-
25902591
/* Restore initial configuration */
25912592
if (data->flags & LM90_HAVE_CONVRATE)
25922593
lm90_write_convrate(data, data->convrate_orig);
25932594
lm90_write_reg(client, LM90_REG_CONFIG1, data->config_orig);
25942595
}
25952596

2597+
static void lm90_stop_work(void *_data)
2598+
{
2599+
struct lm90_data *data = _data;
2600+
2601+
hwmon_lock(data->hwmon_dev);
2602+
data->shutdown = true;
2603+
hwmon_unlock(data->hwmon_dev);
2604+
cancel_delayed_work_sync(&data->alert_work);
2605+
cancel_work_sync(&data->report_work);
2606+
}
2607+
25962608
static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
25972609
{
25982610
struct device_node *np = client->dev.of_node;
@@ -2902,6 +2914,10 @@ static int lm90_probe(struct i2c_client *client)
29022914

29032915
data->hwmon_dev = hwmon_dev;
29042916

2917+
err = devm_add_action_or_reset(&client->dev, lm90_stop_work, data);
2918+
if (err)
2919+
return err;
2920+
29052921
if (client->irq) {
29062922
dev_dbg(dev, "IRQ: %d\n", client->irq);
29072923
err = devm_request_threaded_irq(dev, client->irq,
@@ -2930,7 +2946,7 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
29302946
*/
29312947
struct lm90_data *data = i2c_get_clientdata(client);
29322948

2933-
if ((data->flags & LM90_HAVE_BROKEN_ALERT) &&
2949+
if (!data->shutdown && (data->flags & LM90_HAVE_BROKEN_ALERT) &&
29342950
(data->current_alarms & data->alert_alarms)) {
29352951
if (!(data->config & 0x80)) {
29362952
dev_dbg(&client->dev, "Disabling ALERT#\n");

0 commit comments

Comments
 (0)