re: hwmon: (pmbus) Add support for reading direct mode coefficients

From: Colin Ian King
Date: Thu Jun 10 2021 - 12:55:49 EST


Hi,

Static analysis with Coverity on linux-next has detected a potential
issue in drivers/hwmon/pmbus/pmbus_core.c with the following commit:

commit 999d577d7c007d38ab83eee4532d107c2233f78f
Author: Erik Rosen <erik.rosen@xxxxxxxxxxxxx>
Date: Wed Jun 9 11:32:06 2021 +0200

hwmon: (pmbus) Add support for reading direct mode coefficients

The analysis is as follows:

2219 static int pmbus_init_coefficients(struct i2c_client *client,
2220 struct pmbus_driver_info *info)
2221 {

1. var_decl: Declaring variable ret without initializer.

2222 int i, n, ret;
2223 const struct pmbus_class_attr_map *map;
2224 const struct pmbus_sensor_attr *attr;
2225

2. Condition i < 6UL /* sizeof (class_attr_map) / sizeof
(class_attr_map[0]) + (int)sizeof (struct
pmbus_init_coefficients::[unnamed type]) */, taking true branch.

5. Condition i < 6UL /* sizeof (class_attr_map) / sizeof
(class_attr_map[0]) + (int)sizeof (struct
pmbus_init_coefficients::[unnamed type]) */, taking true branch.

8. Condition i < 6UL /* sizeof (class_attr_map) / sizeof
(class_attr_map[0]) + (int)sizeof (struct
pmbus_init_coefficients::[unnamed type]) */, taking true branch.

2226 for (i = 0; i < ARRAY_SIZE(class_attr_map); i++) {
2227 map = &class_attr_map[i];

3. Condition info->format[map->class] != direct, taking true branch.
6. Condition info->format[map->class] != direct, taking true branch.
9. Condition info->format[map->class] != direct, taking false branch.

2228 if (info->format[map->class] != direct)

4. Continuing loop.
7. Continuing loop.

2229 continue;

10. Condition n < map->nattr, taking true branch.
13. Condition n < map->nattr, taking true branch.
16. Condition n < map->nattr, taking false branch.

2230 for (n = 0; n < map->nattr; n++) {
2231 attr = &map->attr[n];

11. Condition map->class != attr->class, taking true branch.
14. Condition map->class != attr->class, taking true branch.
2232 if (map->class != attr->class)
12. Continuing loop.
15. Continuing loop.

2233 continue;
2234 ret = pmbus_read_coefficients(client, info,
attr);
2235 if (ret >= 0)
2236 break;
2237 }

Uninitialized scalar variable (UNINIT)
17. uninit_use: Using uninitialized value ret.

2238 if (ret < 0) {
2239 dev_err(&client->dev,
2240 "No coefficients found for sensor
class %d\n",
2241 map->class);
2242 return -EINVAL;
2243 }
2244 }
2245
2246 return 0;
2247 }

With the continue statements on line 2233 (or if map->nattr is zero) it
may be possible that ret is never assigned a value and so the check on
line 2238 could be checking an uninitialized variable ret. I'm not sure
if this is a false positive, but it may be worth initializing ret to
some sane value to catch these corner cases.

Colin