linux-kernel - Re: [PATCH v2 2/2] hwmon: Add "label" attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <A1QH5R.BM5NF8G10CEB@crapouillou.net>
Date:   Mon, 10 Jan 2022 10:48:46 +0000
From:   Paul Cercueil <paul@...pouillou.net>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, list@...ndingux.net,
        Cosmin Tanislav <cosmin.tanislav@...log.com>
Subject: Re: [PATCH v2 2/2] hwmon: Add "label" attribute

Hi,

Le dim., janv. 9 2022 at 17:29:24 -0800, Guenter Roeck 
<linux@...ck-us.net> a écrit :
> On Wed, Jan 05, 2022 at 03:15:51PM +0000, Paul Cercueil wrote:
>>  If a label is defined in the device tree for this device add that
>>  to the device specific attributes. This is useful for userspace to
>>  be able to identify an individual device when multiple identical
>>  chips are present in the system.
>> 
>>  Signed-off-by: Paul Cercueil <paul@...pouillou.net>
>>  Tested-by: Cosmin Tanislav <cosmin.tanislav@...log.com>
>>  Reported-by: kernel test robot <lkp@...el.com>
>>  ---
>> 
>>  Notes:
>>      v2: - Cache label into hwmon_device
>>          - Rename hwmon_dev_name_is_visible() to 
>> hwmon_dev_attr_is_visible()
>>  	- Add missing <linux/property.h> include
>> 
>>   drivers/hwmon/hwmon.c | 34 +++++++++++++++++++++++++++++++---
>>   1 file changed, 31 insertions(+), 3 deletions(-)
>> 
>>  diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
>>  index 3501a3ead4ba..22e1b47c09fc 100644
>>  --- a/drivers/hwmon/hwmon.c
>>  +++ b/drivers/hwmon/hwmon.c
>>  @@ -18,6 +18,7 @@
>>   #include <linux/list.h>
>>   #include <linux/module.h>
>>   #include <linux/pci.h>
>>  +#include <linux/property.h>
>>   #include <linux/slab.h>
>>   #include <linux/string.h>
>>   #include <linux/thermal.h>
>>  @@ -30,6 +31,7 @@
>> 
>>   struct hwmon_device {
>>   	const char *name;
>>  +	const char *label;
>>   	struct device dev;
>>   	const struct hwmon_chip_info *chip;
>>   	struct list_head tzdata;
>>  @@ -71,17 +73,29 @@ name_show(struct device *dev, struct 
>> device_attribute *attr, char *buf)
>>   }
>>   static DEVICE_ATTR_RO(name);
>> 
>>  +static ssize_t
>>  +label_show(struct device *dev, struct device_attribute *attr, char 
>> *buf)
>>  +{
>>  +	return sysfs_emit(buf, "%s\n", to_hwmon_device(dev)->label);
>>  +}
>>  +static DEVICE_ATTR_RO(label);
>>  +
>>   static struct attribute *hwmon_dev_attrs[] = {
>>   	&dev_attr_name.attr,
>>  +	&dev_attr_label.attr,
>>   	NULL
>>   };
>> 
>>  -static umode_t hwmon_dev_name_is_visible(struct kobject *kobj,
>>  +static umode_t hwmon_dev_attr_is_visible(struct kobject *kobj,
>>   					 struct attribute *attr, int n)
>>   {
>>   	struct device *dev = kobj_to_dev(kobj);
>>  +	struct hwmon_device *hdev = to_hwmon_device(dev);
>> 
>>  -	if (to_hwmon_device(dev)->name == NULL)
>>  +	if (attr == &dev_attr_name.attr && hdev->name == NULL)
>>  +		return 0;
>>  +
>>  +	if (attr == &dev_attr_label.attr && hdev->label == NULL)
>>   		return 0;
>> 
>>   	return attr->mode;
>>  @@ -89,7 +103,7 @@ static umode_t hwmon_dev_name_is_visible(struct 
>> kobject *kobj,
>> 
>>   static const struct attribute_group hwmon_dev_attr_group = {
>>   	.attrs		= hwmon_dev_attrs,
>>  -	.is_visible	= hwmon_dev_name_is_visible,
>>  +	.is_visible	= hwmon_dev_attr_is_visible,
>>   };
>> 
>>   static const struct attribute_group *hwmon_dev_attr_groups[] = {
>>  @@ -117,6 +131,7 @@ static void hwmon_dev_release(struct device 
>> *dev)
>>   	if (hwdev->group.attrs)
>>   		hwmon_free_attrs(hwdev->group.attrs);
>>   	kfree(hwdev->groups);
>>  +	kfree(hwdev->label);
>>   	kfree(hwdev);
>>   }
>> 
>>  @@ -733,6 +748,7 @@ __hwmon_device_register(struct device *dev, 
>> const char *name, void *drvdata,
>>   			const struct attribute_group **groups)
>>   {
>>   	struct hwmon_device *hwdev;
>>  +	const char *label;
>>   	struct device *hdev;
>>   	int i, err, id;
>> 
>>  @@ -752,6 +768,18 @@ __hwmon_device_register(struct device *dev, 
>> const char *name, void *drvdata,
>>   		goto ida_remove;
>>   	}
>> 
>>  +	if (device_property_present(dev, "label")) {
>>  +		err = device_property_read_string(dev, "label", &label);
>>  +		if (err < 0)
>>  +			goto free_hwmon;
>>  +
>>  +		hwdev->label = kstrdup(label, GFP_KERNEL);
>>  +		if (hwdev->label == NULL) {
>>  +			err = -ENOMEM;
>>  +			goto free_hwmon;
> 
> The code after free_hwmon: uses hdev, so 0-day has a point. Please 
> fix.

Yes indeed. I was going to send a v3.

-Paul

> 
>>  +		}
>>  +	}
>>  +
>>   	hdev = &hwdev->dev;
>> 
>>   	if (chip) {