From: Guenter Roeck <linux@roeck-us.net>
To: Colin Ian King <colin.king@canonical.com>
Cc: Jean Delvare <jdelvare@suse.com>,
linux-hwmon@vger.kernel.org,
Erik Rosen <erik.rosen@metormote.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: hwmon: (pmbus) Add support for reading direct mode coefficients
Date: Thu, 10 Jun 2021 10:14:43 -0700 [thread overview]
Message-ID: <20210610171443.GB3861769@roeck-us.net> (raw)
In-Reply-To: <68276e1d-f262-d379-4600-88abdbecddd8@canonical.com>
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
>
next prev parent reply other threads:[~2021-06-10 17:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 16:55 hwmon: (pmbus) Add support for reading direct mode coefficients Colin Ian King
2021-06-10 17:14 ` Guenter Roeck [this message]
2021-06-10 18:52 ` Erik Rosen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210610171443.GB3861769@roeck-us.net \
--to=linux@roeck-us.net \
--cc=colin.king@canonical.com \
--cc=erik.rosen@metormote.com \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).