* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.