From: Colin Ian King <colin.king@canonical.com>
To: Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
"Greg Kroah-Hartman <gregkh"@linuxfoundation.org,
Vaibhav Agarwal <vaibhav.sr@gmail.com>,
Mark Greer <mgreer@animalcreek.com>,
greybus-dev@lists.linaro.org,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: issue with uninitialized value used in a comparison in gbcodec_mixer_dapm_ctl_put
Date: Thu, 30 Jul 2020 17:02:22 +0100 [thread overview]
Message-ID: <7da6363c-9423-2b9f-029a-395cc8a932d7@canonical.com> (raw)
Hi,
Static analysis with Coverity has detected an uninitialized value being
used in a comparison. The error was detected on a recent change to
drivers/staging/greybus/audio_topology.c however the issue actually
dates back to the original commit:
commit 6339d2322c47f4b8ebabf9daf0130328ed72648b
Author: Vaibhav Agarwal <vaibhav.agarwal@linaro.org>
Date: Wed Jan 13 14:07:51 2016 -0700
greybus: audio: Add topology parser for GB codec
The analysis is as follows:
425 static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol *kcontrol,
426 struct snd_ctl_elem_value
*ucontrol)
427 {
428 int ret, wi, max, connect;
429 unsigned int mask, val;
430 struct gb_audio_ctl_elem_info *info;
431 struct gbaudio_ctl_pvt *data;
1. var_decl: Declaring variable gbvalue without initializer.
432 struct gb_audio_ctl_elem_value gbvalue;
433 struct gbaudio_module_info *module;
434 struct snd_soc_dapm_widget_list *wlist =
snd_kcontrol_chip(kcontrol);
435 struct snd_soc_dapm_widget *widget = wlist->widgets[0];
436 struct device *codec_dev = widget->dapm->dev;
437 struct gbaudio_codec_info *gb = dev_get_drvdata(codec_dev);
438 struct gb_bundle *bundle;
439
2. Condition 0 /* __builtin_types_compatible_p() */, taking false branch.
3. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
4. Falling through to end of if statement.
5. Condition !!branch, taking false branch.
6. Condition ({...; !!branch;}), taking false branch.
440 dev_dbg(codec_dev, "Entered %s:%s\n", __func__,
kcontrol->id.name);
441 module = find_gb_module(gb, kcontrol->id.name);
7. Condition !module, taking false branch.
442 if (!module)
443 return -EINVAL;
444
445 data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
446 info = (struct gb_audio_ctl_elem_info *)data->info;
8. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
447 bundle = to_gb_bundle(module->dev);
448
9. Condition data->vcount == 2, taking true branch.
449 if (data->vcount == 2)
450 dev_warn(widget->dapm->dev,
451 "GB: Control '%s' is stereo, which is not
supported\n",
452 kcontrol->id.name);
453
454 max = le32_to_cpu(info->value.integer.max);
455 mask = (1 << fls(max)) - 1;
456 val = ucontrol->value.integer.value[0] & mask;
10. Condition !!val, taking true branch.
457 connect = !!val;
458
459 /* update ucontrol */
Uninitialized scalar variable (UNINIT)
11. uninit_use: Using uninitialized value gbvalue.value.integer_value[0].
460 if (gbvalue.value.integer_value[0] != val) {
The gbvalue.value.integer_value[0] read is bogus since gbvalue was
declared on the stack but was not initialized. There seems to be no
where that sets this data. I'm assuming most of the time that the
comparison works because the garbage value is different from val and so
the code in the if stanza is executed.
Anyhow, I'm unsure what the original intent of the code was, so I've not
attempted to fix this.
Colin
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
next reply other threads:[~2020-07-30 16:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-30 16:02 Colin Ian King [this message]
2020-07-31 5:08 ` issue with uninitialized value used in a comparison in gbcodec_mixer_dapm_ctl_put Vaibhav Agarwal
2020-08-05 13:35 ` Alex Elder
2020-08-06 16:17 ` Vaibhav Agarwal
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=7da6363c-9423-2b9f-029a-395cc8a932d7@canonical.com \
--to=colin.king@canonical.com \
--cc="Greg Kroah-Hartman <gregkh"@linuxfoundation.org \
--cc=devel@driverdev.osuosl.org \
--cc=elder@kernel.org \
--cc=greybus-dev@lists.linaro.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mgreer@animalcreek.com \
--cc=vaibhav.sr@gmail.com \
/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 NNTP newsgroup(s).