* [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE
@ 2024-07-19 20:55 Dan Carpenter
2024-07-19 21:10 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2024-07-19 20:55 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: linux-hwmon
Hello Eduardo Valentin,
Commit 3aa74796cfd0 ("hwmon: (pmbus) Register with thermal for
PSC_TEMPERATURE") from Apr 28, 2022 (linux-next), leads to the
following Smatch static checker warning:
drivers/hwmon/pmbus/pmbus_core.c:1375 pmbus_add_sensor()
error: we previously assumed 'type' could be null (see line 1347)
drivers/hwmon/pmbus/pmbus_core.c
1331 static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
1332 const char *name, const char *type,
1333 int seq, int page, int phase,
1334 int reg,
1335 enum pmbus_sensor_classes class,
1336 bool update, bool readonly,
1337 bool convert)
1338 {
1339 struct pmbus_sensor *sensor;
1340 struct device_attribute *a;
1341
1342 sensor = devm_kzalloc(data->dev, sizeof(*sensor), GFP_KERNEL);
1343 if (!sensor)
1344 return NULL;
1345 a = &sensor->attribute;
1346
1347 if (type)
^^^^
This code assumes type can be NULL
1348 snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
1349 name, seq, type);
1350 else
1351 snprintf(sensor->name, sizeof(sensor->name), "%s%d",
1352 name, seq);
1353
1354 if (data->flags & PMBUS_WRITE_PROTECTED)
1355 readonly = true;
1356
1357 sensor->page = page;
1358 sensor->phase = phase;
1359 sensor->reg = reg;
1360 sensor->class = class;
1361 sensor->update = update;
1362 sensor->convert = convert;
1363 sensor->data = -ENODATA;
1364 pmbus_dev_attr_init(a, sensor->name,
1365 readonly ? 0444 : 0644,
1366 pmbus_show_sensor, pmbus_set_sensor);
1367
1368 if (pmbus_add_attribute(data, &a->attr))
1369 return NULL;
1370
1371 sensor->next = data->sensors;
1372 data->sensors = sensor;
1373
1374 /* temperature sensors with _input values are registered with thermal */
--> 1375 if (class == PSC_TEMPERATURE && strcmp(type, "input") == 0)
^^^^
Unchecked dereference
1376 pmbus_thermal_add_sensor(data, sensor, seq);
1377
1378 return sensor;
1379 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE
2024-07-19 20:55 [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE Dan Carpenter
@ 2024-07-19 21:10 ` Guenter Roeck
2024-07-19 22:33 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2024-07-19 21:10 UTC (permalink / raw)
To: Dan Carpenter, Eduardo Valentin; +Cc: linux-hwmon
On 7/19/24 13:55, Dan Carpenter wrote:
> Hello Eduardo Valentin,
>
> Commit 3aa74796cfd0 ("hwmon: (pmbus) Register with thermal for
> PSC_TEMPERATURE") from Apr 28, 2022 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/hwmon/pmbus/pmbus_core.c:1375 pmbus_add_sensor()
> error: we previously assumed 'type' could be null (see line 1347)
>
> drivers/hwmon/pmbus/pmbus_core.c
> 1331 static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
> 1332 const char *name, const char *type,
> 1333 int seq, int page, int phase,
> 1334 int reg,
> 1335 enum pmbus_sensor_classes class,
> 1336 bool update, bool readonly,
> 1337 bool convert)
> 1338 {
> 1339 struct pmbus_sensor *sensor;
> 1340 struct device_attribute *a;
> 1341
> 1342 sensor = devm_kzalloc(data->dev, sizeof(*sensor), GFP_KERNEL);
> 1343 if (!sensor)
> 1344 return NULL;
> 1345 a = &sensor->attribute;
> 1346
> 1347 if (type)
> ^^^^
> This code assumes type can be NULL
>
> 1348 snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
> 1349 name, seq, type);
> 1350 else
> 1351 snprintf(sensor->name, sizeof(sensor->name), "%s%d",
> 1352 name, seq);
> 1353
> 1354 if (data->flags & PMBUS_WRITE_PROTECTED)
> 1355 readonly = true;
> 1356
> 1357 sensor->page = page;
> 1358 sensor->phase = phase;
> 1359 sensor->reg = reg;
> 1360 sensor->class = class;
> 1361 sensor->update = update;
> 1362 sensor->convert = convert;
> 1363 sensor->data = -ENODATA;
> 1364 pmbus_dev_attr_init(a, sensor->name,
> 1365 readonly ? 0444 : 0644,
> 1366 pmbus_show_sensor, pmbus_set_sensor);
> 1367
> 1368 if (pmbus_add_attribute(data, &a->attr))
> 1369 return NULL;
> 1370
> 1371 sensor->next = data->sensors;
> 1372 data->sensors = sensor;
> 1373
> 1374 /* temperature sensors with _input values are registered with thermal */
> --> 1375 if (class == PSC_TEMPERATURE && strcmp(type, "input") == 0)
> ^^^^
> Unchecked dereference
>
It is only NULL for PSC_PWM, never for PSC_TEMPERATURE. We could add a check to
make the static checker happy but it won't make a practical difference.
Guenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE
2024-07-19 21:10 ` Guenter Roeck
@ 2024-07-19 22:33 ` Dan Carpenter
2024-07-29 21:16 ` Eduardo Valentin
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2024-07-19 22:33 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Eduardo Valentin, linux-hwmon
On Fri, Jul 19, 2024 at 02:10:16PM -0700, Guenter Roeck wrote:
> > 1373
> > 1374 /* temperature sensors with _input values are registered with thermal */
> > --> 1375 if (class == PSC_TEMPERATURE && strcmp(type, "input") == 0)
> > ^^^^
> > Unchecked dereference
> >
>
> It is only NULL for PSC_PWM, never for PSC_TEMPERATURE. We could add a check to
> make the static checker happy but it won't make a practical difference.
No, don't do that. Just ignore the warning in that case.
I'm running with the cross function database, and in theory that should
have silenced the warning. I'll investigate.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE
2024-07-19 22:33 ` Dan Carpenter
@ 2024-07-29 21:16 ` Eduardo Valentin
0 siblings, 0 replies; 4+ messages in thread
From: Eduardo Valentin @ 2024-07-29 21:16 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Guenter Roeck, Eduardo Valentin, linux-hwmon
Hello,
On Fri, Jul 19, 2024 at 05:33:19PM -0500, Dan Carpenter wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Fri, Jul 19, 2024 at 02:10:16PM -0700, Guenter Roeck wrote:
> > > 1373
> > > 1374 /* temperature sensors with _input values are registered with thermal */
> > > --> 1375 if (class == PSC_TEMPERATURE && strcmp(type, "input") == 0)
> > > ^^^^
> > > Unchecked dereference
> > >
> >
> > It is only NULL for PSC_PWM, never for PSC_TEMPERATURE. We could add a check to
> > make the static checker happy but it won't make a practical difference.
>
> No, don't do that. Just ignore the warning in that case.
>
> I'm running with the cross function database, and in theory that should
> have silenced the warning. I'll investigate.
Nice you are doing this, Dan.
Yeah. Let me know the outcome of your investigation. I can always patch it up.
Thanks.
>
>
> regards,
> dan carpenter
>
--
All the best,
Eduardo Valentin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-29 21:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-19 20:55 [bug report] hwmon: (pmbus) Register with thermal for PSC_TEMPERATURE Dan Carpenter
2024-07-19 21:10 ` Guenter Roeck
2024-07-19 22:33 ` Dan Carpenter
2024-07-29 21:16 ` Eduardo Valentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox