* [bug report] hwmon: Add "label" attribute
@ 2022-01-27 8:52 Dan Carpenter
2022-01-27 9:47 ` Paul Cercueil
2022-01-31 16:27 ` [PATCH] hwmon: Fix possible NULL pointer Paul Cercueil
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-01-27 8:52 UTC (permalink / raw)
To: paul; +Cc: linux-hwmon
Hello Paul Cercueil,
This is a semi-automatic email about new static checker warnings.
The patch 073c3ea6c530: "hwmon: Add "label" attribute" from Jan 10,
2022, leads to the following Smatch complaint:
drivers/hwmon/hwmon.c:825 __hwmon_device_register()
warn: variable dereferenced before check 'dev' (see line 810)
drivers/hwmon/hwmon.c
809
810 if (device_property_present(dev, "label")) {
^^^
The patch adds a new unchecked dereference
811 err = device_property_read_string(dev, "label", &label);
812 if (err < 0)
813 goto free_hwmon;
814
815 hwdev->label = kstrdup(label, GFP_KERNEL);
816 if (hwdev->label == NULL) {
817 err = -ENOMEM;
818 goto free_hwmon;
819 }
820 }
821
822 hwdev->name = name;
823 hdev->class = &hwmon_class;
824 hdev->parent = dev;
825 hdev->of_node = dev ? dev->of_node : NULL;
^^^
Existing code checked for NULL
826 hwdev->chip = chip;
827 dev_set_drvdata(hdev, drvdata);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] hwmon: Add "label" attribute
2022-01-27 8:52 [bug report] hwmon: Add "label" attribute Dan Carpenter
@ 2022-01-27 9:47 ` Paul Cercueil
2022-01-27 14:28 ` Guenter Roeck
2022-01-31 16:27 ` [PATCH] hwmon: Fix possible NULL pointer Paul Cercueil
1 sibling, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2022-01-27 9:47 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-hwmon
Hi,
Le jeu., janv. 27 2022 at 11:52:45 +0300, Dan Carpenter
<dan.carpenter@oracle.com> a écrit :
> Hello Paul Cercueil,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 073c3ea6c530: "hwmon: Add "label" attribute" from Jan 10,
> 2022, leads to the following Smatch complaint:
>
> drivers/hwmon/hwmon.c:825 __hwmon_device_register()
> warn: variable dereferenced before check 'dev' (see line 810)
>
> drivers/hwmon/hwmon.c
> 809
> 810 if (device_property_present(dev, "label")) {
> ^^^
> The patch adds a new unchecked dereference
I will send a patch to address that.
I'm surprised that this function can be called with dev == NULL in the
first place, though.
Cheers,
-Paul
> 811 err = device_property_read_string(dev, "label", &label);
> 812 if (err < 0)
> 813 goto free_hwmon;
> 814
> 815 hwdev->label = kstrdup(label, GFP_KERNEL);
> 816 if (hwdev->label == NULL) {
> 817 err = -ENOMEM;
> 818 goto free_hwmon;
> 819 }
> 820 }
> 821
> 822 hwdev->name = name;
> 823 hdev->class = &hwmon_class;
> 824 hdev->parent = dev;
> 825 hdev->of_node = dev ? dev->of_node : NULL;
> ^^^
> Existing code checked for NULL
>
> 826 hwdev->chip = chip;
> 827 dev_set_drvdata(hdev, drvdata);
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] hwmon: Add "label" attribute
2022-01-27 9:47 ` Paul Cercueil
@ 2022-01-27 14:28 ` Guenter Roeck
2022-01-27 16:43 ` Paul Cercueil
0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2022-01-27 14:28 UTC (permalink / raw)
To: Paul Cercueil, Dan Carpenter; +Cc: linux-hwmon
On 1/27/22 01:47, Paul Cercueil wrote:
> Hi,
>
> Le jeu., janv. 27 2022 at 11:52:45 +0300, Dan Carpenter <dan.carpenter@oracle.com> a écrit :
>> Hello Paul Cercueil,
>>
>> This is a semi-automatic email about new static checker warnings.
>>
>> The patch 073c3ea6c530: "hwmon: Add "label" attribute" from Jan 10,
>> 2022, leads to the following Smatch complaint:
>>
>> drivers/hwmon/hwmon.c:825 __hwmon_device_register()
>> warn: variable dereferenced before check 'dev' (see line 810)
>>
>> drivers/hwmon/hwmon.c
>> 809
>> 810 if (device_property_present(dev, "label")) {
>> ^^^
>> The patch adds a new unchecked dereference
>
> I will send a patch to address that.
>
> I'm surprised that this function can be called with dev == NULL in the first place, though.
>
Originally it was needed for the thermal subsystem, which did not provide a parent
device. By the time that was reworked, it was (mis-)used by the Loongson-3 hwmon
driver (which was never reviewed by a hwmon maintainer and does pretty much
everything wrong).
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] hwmon: Add "label" attribute
2022-01-27 14:28 ` Guenter Roeck
@ 2022-01-27 16:43 ` Paul Cercueil
2022-01-27 17:17 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2022-01-27 16:43 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Dan Carpenter, linux-hwmon
Hi Guenter,
Le jeu., janv. 27 2022 at 06:28:26 -0800, Guenter Roeck
<linux@roeck-us.net> a écrit :
> On 1/27/22 01:47, Paul Cercueil wrote:
>> Hi,
>>
>> Le jeu., janv. 27 2022 at 11:52:45 +0300, Dan Carpenter
>> <dan.carpenter@oracle.com> a écrit :
>>> Hello Paul Cercueil,
>>>
>>> This is a semi-automatic email about new static checker warnings.
>>>
>>> The patch 073c3ea6c530: "hwmon: Add "label" attribute" from Jan 10,
>>> 2022, leads to the following Smatch complaint:
>>>
>>> drivers/hwmon/hwmon.c:825 __hwmon_device_register()
>>> warn: variable dereferenced before check 'dev' (see line 810)
>>>
>>> drivers/hwmon/hwmon.c
>>> 809
>>> 810 if (device_property_present(dev, "label")) {
>>> ^^^
>>> The patch adds a new unchecked dereference
>>
>> I will send a patch to address that.
>>
>> I'm surprised that this function can be called with dev == NULL in
>> the first place, though.
>>
>
> Originally it was needed for the thermal subsystem, which did not
> provide a parent
> device. By the time that was reworked, it was (mis-)used by the
> Loongson-3 hwmon
> driver (which was never reviewed by a hwmon maintainer and does
> pretty much
> everything wrong).
Where is that Loongson-3 hwmon driver? I can't find it anywhere.
Maybe we can change that now?
-Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] hwmon: Add "label" attribute
2022-01-27 16:43 ` Paul Cercueil
@ 2022-01-27 17:17 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2022-01-27 17:17 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Dan Carpenter, linux-hwmon
On 1/27/22 08:43, Paul Cercueil wrote:
> Hi Guenter,
>
> Le jeu., janv. 27 2022 at 06:28:26 -0800, Guenter Roeck <linux@roeck-us.net> a écrit :
>> On 1/27/22 01:47, Paul Cercueil wrote:
>>> Hi,
>>>
>>> Le jeu., janv. 27 2022 at 11:52:45 +0300, Dan Carpenter <dan.carpenter@oracle.com> a écrit :
>>>> Hello Paul Cercueil,
>>>>
>>>> This is a semi-automatic email about new static checker warnings.
>>>>
>>>> The patch 073c3ea6c530: "hwmon: Add "label" attribute" from Jan 10,
>>>> 2022, leads to the following Smatch complaint:
>>>>
>>>> drivers/hwmon/hwmon.c:825 __hwmon_device_register()
>>>> warn: variable dereferenced before check 'dev' (see line 810)
>>>>
>>>> drivers/hwmon/hwmon.c
>>>> 809
>>>> 810 if (device_property_present(dev, "label")) {
>>>> ^^^
>>>> The patch adds a new unchecked dereference
>>>
>>> I will send a patch to address that.
>>>
>>> I'm surprised that this function can be called with dev == NULL in the first place, though.
>>>
>>
>> Originally it was needed for the thermal subsystem, which did not provide a parent
>> device. By the time that was reworked, it was (mis-)used by the Loongson-3 hwmon
>> driver (which was never reviewed by a hwmon maintainer and does pretty much
>> everything wrong).
>
> Where is that Loongson-3 hwmon driver? I can't find it anywhere.
>
drivers/platform/mips/cpu_hwmon.c
> Maybe we can change that now?
>
It should be a platform driver, it should only instantiate on hardware supporting it,
it should leave the name attribute alone, it should not generate its sysfs attributes
but use hwmon_channel_info / hwmon_chip_info / hwmon_ops, and it should use the
is_visible callback in struct hwmon_ops lm90_ops to determine if attributes are
visible. This is just the problems I noticed after a few minutes of looking into
the code; there may be more. This would be a lot of work, with no means to test
the result.
It might make more sense to add a warning to the hwmon core if dev is NULL.
We should also have a warning in hwmon_device_register_with_info() if the struct
hwmon_chip_info pointer is NULL (the API should really not be used in that case),
but that would require changing the thermal code to use with_groups(). Even that
would be less than perfect since it still lets people abuse the with_groups API
(calling hwmon_device_register_with_groups with NULL groups pointer does not
really make sense either). Given the nature of the thermal code, I don't know if
it would even be possible to fix that.
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] hwmon: Fix possible NULL pointer
2022-01-27 8:52 [bug report] hwmon: Add "label" attribute Dan Carpenter
2022-01-27 9:47 ` Paul Cercueil
@ 2022-01-31 16:27 ` Paul Cercueil
1 sibling, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2022-01-31 16:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Guenter Roeck, linux-hwmon, Paul Cercueil
The recent addition of the label attribute added some code that read the
"label" device property, without checking first that "dev" was non-NULL.
Fix this issue by first checking that "dev" is non-NULL.
Fixes: ccd98cba6a18 ("hwmon: Add "label" attribute")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/hwmon/hwmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index e36ea82da147..5915fedee69b 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -807,7 +807,7 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
hdev->groups = groups;
}
- if (device_property_present(dev, "label")) {
+ if (dev && device_property_present(dev, "label")) {
err = device_property_read_string(dev, "label", &label);
if (err < 0)
goto free_hwmon;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-31 16:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-27 8:52 [bug report] hwmon: Add "label" attribute Dan Carpenter
2022-01-27 9:47 ` Paul Cercueil
2022-01-27 14:28 ` Guenter Roeck
2022-01-27 16:43 ` Paul Cercueil
2022-01-27 17:17 ` Guenter Roeck
2022-01-31 16:27 ` [PATCH] hwmon: Fix possible NULL pointer Paul Cercueil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox