On 1/27/22 08:43, Paul Cercueil wrote:
Hi Guenter,
Le jeu., janv. 27 2022 at 06:28:26 -0800, Guenter Roeck <linux@xxxxxxxxxxxx> 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@xxxxxxxxxx> 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
