linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* re: hwmon: (pmbus) Add support for reading direct mode coefficients
@ 2021-06-10 16:55 Colin Ian King
  2021-06-10 17:14 ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: Colin Ian King @ 2021-06-10 16:55 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare, linux-hwmon, Erik Rosen; +Cc: linux-kernel

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@metormote.com>
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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: hwmon: (pmbus) Add support for reading direct mode coefficients
  2021-06-10 16:55 hwmon: (pmbus) Add support for reading direct mode coefficients Colin Ian King
@ 2021-06-10 17:14 ` Guenter Roeck
  2021-06-10 18:52   ` Erik Rosen
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2021-06-10 17:14 UTC (permalink / raw)
  To: Colin Ian King; +Cc: Jean Delvare, linux-hwmon, Erik Rosen, linux-kernel

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@metormote.com>
> 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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: hwmon: (pmbus) Add support for reading direct mode coefficients
  2021-06-10 17:14 ` Guenter Roeck
@ 2021-06-10 18:52   ` Erik Rosen
  0 siblings, 0 replies; 3+ messages in thread
From: Erik Rosen @ 2021-06-10 18:52 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Colin Ian King, Jean Delvare, linux-hwmon, linux-kernel

Ok, thanks.

/Erik

On Thu, Jun 10, 2021 at 7:15 PM Guenter Roeck <linux@roeck-us.net> 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@metormote.com>
> > 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
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-10 18:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 16:55 hwmon: (pmbus) Add support for reading direct mode coefficients Colin Ian King
2021-06-10 17:14 ` Guenter Roeck
2021-06-10 18:52   ` Erik Rosen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).