Skip to content

Commit

Permalink
hwmon: (core) Do not use device managed functions for memory allocations
Browse files Browse the repository at this point in the history 
The hwmon core uses device managed functions, tied to the hwmon parent
device, for various internal memory allocations. This is problematic
since hwmon device lifetime does not necessarily match its parent's
device lifetime. If there is a mismatch, memory leaks will accumulate
until the parent device is released.

Fix the problem by managing all memory allocations internally. The only
exception is memory allocation for thermal device registration, which
can be tied to the hwmon device, along with thermal device registration
itself.

Fixes: d560168 ("hwmon: (core) New hwmon registration API")
Cc: stable@vger.kernel.org # v4.14.x: 47c332d: hwmon: Deal with errors from the thermal subsystem
Cc: stable@vger.kernel.org # v4.14.x: 74e3512: hwmon: (core) Fix double-free in __hwmon_device_register()
Cc: stable@vger.kernel.org # v4.9.x: 3a412d5: hwmon: (core) Simplify sysfs attribute name allocation
Cc: stable@vger.kernel.org # v4.9.x: 47c332d: hwmon: Deal with errors from the thermal subsystem
Cc: stable@vger.kernel.org # v4.9.x: 74e3512: hwmon: (core) Fix double-free in __hwmon_device_register()
Cc: stable@vger.kernel.org # v4.9+
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
  • Loading branch information
Guenter Roeck committed Jan 17, 2020
1 parent cf3ca18 commit 3bf8bdc
Showing 1 changed file with 41 additions and 27 deletions.
68 changes: 41 additions & 27 deletions drivers/hwmon/hwmon.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@ struct hwmon_device_attribute {

#define to_hwmon_attr(d) \
container_of(d, struct hwmon_device_attribute, dev_attr)
#define to_dev_attr(a) container_of(a, struct device_attribute, attr)

/*
* Thermal zone information
* In addition to the reference to the hwmon device,
* also provides the sensor index.
*/
struct hwmon_thermal_data {
struct hwmon_device *hwdev; /* Reference to hwmon device */
struct device *dev; /* Reference to hwmon device */
int index; /* sensor index */
};

Expand Down Expand Up @@ -95,9 +96,27 @@ static const struct attribute_group *hwmon_dev_attr_groups[] = {
NULL
};

static void hwmon_free_attrs(struct attribute **attrs)
{
int i;

for (i = 0; attrs[i]; i++) {
struct device_attribute *dattr = to_dev_attr(attrs[i]);
struct hwmon_device_attribute *hattr = to_hwmon_attr(dattr);

kfree(hattr);
}
kfree(attrs);
}

static void hwmon_dev_release(struct device *dev)
{
kfree(to_hwmon_device(dev));
struct hwmon_device *hwdev = to_hwmon_device(dev);

if (hwdev->group.attrs)
hwmon_free_attrs(hwdev->group.attrs);
kfree(hwdev->groups);
kfree(hwdev);
}

static struct class hwmon_class = {
Expand All @@ -119,11 +138,11 @@ static DEFINE_IDA(hwmon_ida);
static int hwmon_thermal_get_temp(void *data, int *temp)
{
struct hwmon_thermal_data *tdata = data;
struct hwmon_device *hwdev = tdata->hwdev;
struct hwmon_device *hwdev = to_hwmon_device(tdata->dev);
int ret;
long t;

ret = hwdev->chip->ops->read(&hwdev->dev, hwmon_temp, hwmon_temp_input,
ret = hwdev->chip->ops->read(tdata->dev, hwmon_temp, hwmon_temp_input,
tdata->index, &t);
if (ret < 0)
return ret;
Expand All @@ -137,8 +156,7 @@ static const struct thermal_zone_of_device_ops hwmon_thermal_ops = {
.get_temp = hwmon_thermal_get_temp,
};

static int hwmon_thermal_add_sensor(struct device *dev,
struct hwmon_device *hwdev, int index)
static int hwmon_thermal_add_sensor(struct device *dev, int index)
{
struct hwmon_thermal_data *tdata;
struct thermal_zone_device *tzd;
Expand All @@ -147,10 +165,10 @@ static int hwmon_thermal_add_sensor(struct device *dev,
if (!tdata)
return -ENOMEM;

tdata->hwdev = hwdev;
tdata->dev = dev;
tdata->index = index;

tzd = devm_thermal_zone_of_sensor_register(&hwdev->dev, index, tdata,
tzd = devm_thermal_zone_of_sensor_register(dev, index, tdata,
&hwmon_thermal_ops);
/*
* If CONFIG_THERMAL_OF is disabled, this returns -ENODEV,
Expand All @@ -162,8 +180,7 @@ static int hwmon_thermal_add_sensor(struct device *dev,
return 0;
}
#else
static int hwmon_thermal_add_sensor(struct device *dev,
struct hwmon_device *hwdev, int index)
static int hwmon_thermal_add_sensor(struct device *dev, int index)
{
return 0;
}
Expand Down Expand Up @@ -250,8 +267,7 @@ static bool is_string_attr(enum hwmon_sensor_types type, u32 attr)
(type == hwmon_fan && attr == hwmon_fan_label);
}

static struct attribute *hwmon_genattr(struct device *dev,
const void *drvdata,
static struct attribute *hwmon_genattr(const void *drvdata,
enum hwmon_sensor_types type,
u32 attr,
int index,
Expand Down Expand Up @@ -279,7 +295,7 @@ static struct attribute *hwmon_genattr(struct device *dev,
if ((mode & 0222) && !ops->write)
return ERR_PTR(-EINVAL);

hattr = devm_kzalloc(dev, sizeof(*hattr), GFP_KERNEL);
hattr = kzalloc(sizeof(*hattr), GFP_KERNEL);
if (!hattr)
return ERR_PTR(-ENOMEM);

Expand Down Expand Up @@ -492,8 +508,7 @@ static int hwmon_num_channel_attrs(const struct hwmon_channel_info *info)
return n;
}

static int hwmon_genattrs(struct device *dev,
const void *drvdata,
static int hwmon_genattrs(const void *drvdata,
struct attribute **attrs,
const struct hwmon_ops *ops,
const struct hwmon_channel_info *info)
Expand All @@ -519,7 +534,7 @@ static int hwmon_genattrs(struct device *dev,
attr_mask &= ~BIT(attr);
if (attr >= template_size)
return -EINVAL;
a = hwmon_genattr(dev, drvdata, info->type, attr, i,
a = hwmon_genattr(drvdata, info->type, attr, i,
templates[attr], ops);
if (IS_ERR(a)) {
if (PTR_ERR(a) != -ENOENT)
Expand All @@ -533,8 +548,7 @@ static int hwmon_genattrs(struct device *dev,
}

static struct attribute **
__hwmon_create_attrs(struct device *dev, const void *drvdata,
const struct hwmon_chip_info *chip)
__hwmon_create_attrs(const void *drvdata, const struct hwmon_chip_info *chip)
{
int ret, i, aindex = 0, nattrs = 0;
struct attribute **attrs;
Expand All @@ -545,15 +559,17 @@ __hwmon_create_attrs(struct device *dev, const void *drvdata,
if (nattrs == 0)
return ERR_PTR(-EINVAL);

attrs = devm_kcalloc(dev, nattrs + 1, sizeof(*attrs), GFP_KERNEL);
attrs = kcalloc(nattrs + 1, sizeof(*attrs), GFP_KERNEL);
if (!attrs)
return ERR_PTR(-ENOMEM);

for (i = 0; chip->info[i]; i++) {
ret = hwmon_genattrs(dev, drvdata, &attrs[aindex], chip->ops,
ret = hwmon_genattrs(drvdata, &attrs[aindex], chip->ops,
chip->info[i]);
if (ret < 0)
if (ret < 0) {
hwmon_free_attrs(attrs);
return ERR_PTR(ret);
}
aindex += ret;
}

Expand Down Expand Up @@ -595,14 +611,13 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
for (i = 0; groups[i]; i++)
ngroups++;

hwdev->groups = devm_kcalloc(dev, ngroups, sizeof(*groups),
GFP_KERNEL);
hwdev->groups = kcalloc(ngroups, sizeof(*groups), GFP_KERNEL);
if (!hwdev->groups) {
err = -ENOMEM;
goto free_hwmon;
}

attrs = __hwmon_create_attrs(dev, drvdata, chip);
attrs = __hwmon_create_attrs(drvdata, chip);
if (IS_ERR(attrs)) {
err = PTR_ERR(attrs);
goto free_hwmon;
Expand Down Expand Up @@ -647,8 +662,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
hwmon_temp_input, j))
continue;
if (info[i]->config[j] & HWMON_T_INPUT) {
err = hwmon_thermal_add_sensor(dev,
hwdev, j);
err = hwmon_thermal_add_sensor(hdev, j);
if (err) {
device_unregister(hdev);
/*
Expand All @@ -667,7 +681,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
return hdev;

free_hwmon:
kfree(hwdev);
hwmon_dev_release(hdev);
ida_remove:
ida_simple_remove(&hwmon_ida, id);
return ERR_PTR(err);
Expand Down

0 comments on commit 3bf8bdc

Please sign in to comment.