Re: [PATCH 5/5] hwmon: (drivetemp) Use devres function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux