Thanks for pointing out, I think you are right.
--
Jackie Liu
在 2021/12/22 上午11:03, Guenter Roeck 写道:
On 12/21/21 6:01 PM, Jackie Liu wrote:
From: Jackie Liu <liuyun01@xxxxxxxxxx>
Use devm_hwmon_device_register_with_info() and remove hwmon_dev
from drivetemp_data struct as it is not needed anymore.
Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx>
---
drivers/hwmon/drivetemp.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
index 1eb37106a220..1a5a62288c0a 100644
--- a/drivers/hwmon/drivetemp.c
+++ b/drivers/hwmon/drivetemp.c
@@ -113,7 +113,6 @@ struct drivetemp_data {
struct mutex lock; /* protect data buffer accesses */
struct scsi_device *sdev; /* SCSI device */
struct device *dev; /* instantiating device */
- struct device *hwdev; /* hardware monitoring device */
u8 smartdata[ATA_SECT_SIZE]; /* local buffer */
int (*get_temp)(struct drivetemp_data *st, u32 attr, long *val);
bool have_temp_lowest; /* lowest temp in SCT status */
@@ -555,6 +554,7 @@ static int drivetemp_add(struct device *dev,
struct class_interface *intf)
{
struct scsi_device *sdev = to_scsi_device(dev->parent);
struct drivetemp_data *st;
+ struct device *hwmon_dev;
int err;
st = kzalloc(sizeof(*st), GFP_KERNEL);
@@ -570,13 +570,13 @@ static int drivetemp_add(struct device *dev,
struct class_interface *intf)
goto abort;
}
- st->hwdev = hwmon_device_register_with_info(dev->parent,
"drivetemp",
- st, &drivetemp_chip_info,
- NULL);
- if (IS_ERR(st->hwdev)) {
- err = PTR_ERR(st->hwdev);
+ hwmon_dev =
+ devm_hwmon_device_register_with_info(dev->parent, "drivetemp",
+ st, &drivetemp_chip_info,
+ NULL);
+ err = PTR_ERR_OR_ZERO(hwmon_dev);
+ if (err)
goto abort;
- }
list_add(&st->list, &drivetemp_devlist);
return 0;
@@ -593,7 +593,6 @@ static void drivetemp_remove(struct device *dev,
struct class_interface *intf)
list_for_each_entry_safe(st, tmp, &drivetemp_devlist, list) {
if (st->dev == dev) {
list_del(&st->list);
- hwmon_device_unregister(st->hwdev);
kfree(st);
break;
}
With this change in place, hwmon devices are removed _after_ struct
drivetemp_data
is released, which means that there is a race condition where hwmon
access is still
possible and uses a released data structure. Besides, it is not well
defined if
the lifetime of the hwmon device matches the lifetime of the parent
device. I don't
think you know what you are doing, sorry, and I am not even going to
look into the
other patches of this series.
Guenter
