Ok, thanks.
/Erik
On Thu, Jun 10, 2021 at 7:15 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> Eric,
>
> On Thu, Jun 10, 2021 at 05:55:40PM +0100, Colin Ian King wrote:
> > Hi,
> >
> > Static analysis with Coverity on linux-next has detected a potential
> > issue in drivers/hwmon/pmbus/pmbus_core.c with the following commit:
> >
> No need to send a patch - I fixed it up by pre-initializing ret.
>
> Thanks,
> Guenter
>
> > 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
> >
