drivers/hwmon/fschmd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
fschmd_remove() can detach the i2c client while the watchdog device
is still open.
If watchdog_release() later takes the unexpected-close path, it calls
watchdog_trigger() and then logs the event with dev_crit() using
data->client->dev. However, the client may already have been cleared
by the remove path.
watchdog_trigger() already checks whether data->client is still valid
and returns -ENODEV if the client is gone. Use its return value to
decide whether to emit the message, and log with watchdog_name instead
of dereferencing data->client->dev.
This avoids relying on a device pointer that may no longer be valid
during release after device removal.
Signed-off-by: Chen-Shi-Hong <eric039eric@gmail.com>
---
drivers/hwmon/fschmd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/fschmd.c b/drivers/hwmon/fschmd.c
index 1211fa2259e5..d1e7fc72e81f 100644
--- a/drivers/hwmon/fschmd.c
+++ b/drivers/hwmon/fschmd.c
@@ -834,9 +834,9 @@ static int watchdog_release(struct inode *inode, struct file *filp)
watchdog_stop(data);
data->watchdog_expect_close = 0;
} else {
- watchdog_trigger(data);
- dev_crit(&data->client->dev,
- "unexpected close, not stopping watchdog!\n");
+ if (!watchdog_trigger(data))
+ pr_crit("%s: unexpected close, not stopping watchdog!\n",
+ data->watchdog_name);
}
clear_bit(0, &data->watchdog_is_open);
--
2.53.0
On 5/22/26 22:10, Chen-Shi-Hong wrote:
> fschmd_remove() can detach the i2c client while the watchdog device
> is still open.
>
> If watchdog_release() later takes the unexpected-close path, it calls
> watchdog_trigger() and then logs the event with dev_crit() using
> data->client->dev. However, the client may already have been cleared
> by the remove path.
>
> watchdog_trigger() already checks whether data->client is still valid
> and returns -ENODEV if the client is gone. Use its return value to
> decide whether to emit the message, and log with watchdog_name instead
> of dereferencing data->client->dev.
>
> This avoids relying on a device pointer that may no longer be valid
> during release after device removal.
>
That driver has lots of problems, which would best be fixed by converting its
watchdog driver to a real watchdog driver and actually fixing the other real
problems, such as missing i2c error checks or the real existing race conditions.
This patch is like bandaging a blemish on a finger while ignoring that the leg
is half torn off.
Please, please, please, stop sending such patches. You bury the real important
fixes in the noise.
Guenter
> Signed-off-by: Chen-Shi-Hong <eric039eric@gmail.com>
> ---
> drivers/hwmon/fschmd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/fschmd.c b/drivers/hwmon/fschmd.c
> index 1211fa2259e5..d1e7fc72e81f 100644
> --- a/drivers/hwmon/fschmd.c
> +++ b/drivers/hwmon/fschmd.c
> @@ -834,9 +834,9 @@ static int watchdog_release(struct inode *inode, struct file *filp)
> watchdog_stop(data);
> data->watchdog_expect_close = 0;
> } else {
> - watchdog_trigger(data);
> - dev_crit(&data->client->dev,
> - "unexpected close, not stopping watchdog!\n");
> + if (!watchdog_trigger(data))
> + pr_crit("%s: unexpected close, not stopping watchdog!\n",
> + data->watchdog_name);
> }
>
> clear_bit(0, &data->watchdog_is_open);
Hi Guenter, Understood, and thank you for the direct feedback. I see your point that this patch does not address the real problems in the driver and only adds noise. I won't pursue this kind of change for fschmd further. Thanks, Chen-Shi-Hong
© 2016 - 2026 Red Hat, Inc.