* [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