[PATCH] hwmon: (fschmd) avoid client->dev dereference in watchdog_release

[PATCH] hwmon: (fschmd) avoid client->dev dereference in watchdog_release

[Chen-Shi-Hong]

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

Re: [PATCH] hwmon: (fschmd) avoid client->dev dereference in watchdog_release

[Guenter Roeck]

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);

Re: [PATCH] hwmon: (fschmd) avoid client->dev dereference in watchdog_release

[Chen-Shi-Hong]

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