From: Nathan Chancellor <nathan@kernel.org>
To: Colin Ian King <colin.king@canonical.com>
Cc: Mark Brown <broonie@kernel.org>,
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
"Banajit Goswami <bgoswami"@codeaurora.org,
Liam Girdwood <lgirdwood@gmail.com>,
Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
alsa-devel@alsa-project.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Yang Li <yang.lee@linux.alibaba.com>
Subject: Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
Date: Wed, 12 May 2021 14:15:28 -0700 [thread overview]
Message-ID: <7b2d1562-3629-8cd3-d9a7-926c93d20e08@kernel.org> (raw)
In-Reply-To: <5935b287-5b74-84ac-3470-3b71fd69c88c@canonical.com>
On 5/12/2021 1:12 PM, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity on linux-next has detected an issue with
> the following commit:
>
> commit 5f1b95d08de712327e452d082a50fded435ec884
> Author: Yang Li <yang.lee@linux.alibaba.com>
> Date: Sun Apr 25 18:12:33 2021 +0800
>
> ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
>
> the analysis is as follows:
>
> 1181 int q6afe_port_stop(struct q6afe_port *port)
> 1182 {
> 1183 struct afe_port_cmd_device_stop *stop;
> 1184 struct q6afe *afe = port->afe;
> 1185 struct apr_pkt *pkt;
>
> 1. var_decl: Declaring variable port_id without initializer.
>
> 1186 int port_id;
> 1187 int ret = 0;
> 1188 int index, pkt_size;
> 1189 void *p;
> 1190
> 1191 index = port->token;
>
> 2. Condition index < 0, taking false branch.
> 3. Condition index >= 127, taking false branch.
>
> 1192 if (index < 0 || index >= AFE_PORT_MAX) {
> 1193 dev_err(afe->dev, "AFE port index[%d] invalid!\n",
> index);
> 1194 return -EINVAL;
> 1195 }
> 1196
> 1197 pkt_size = APR_HDR_SIZE + sizeof(*stop);
> 1198 p = kzalloc(pkt_size, GFP_KERNEL);
>
> 4. Condition !p, taking false branch.
>
> 1199 if (!p)
> 1200 return -ENOMEM;
> 1201
> 1202 pkt = p;
> 1203 stop = p + APR_HDR_SIZE;
> 1204
> 1205 pkt->hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
> 1206 APR_HDR_LEN(APR_HDR_SIZE),
> 1207 APR_PKT_VER);
> 1208 pkt->hdr.pkt_size = pkt_size;
> 1209 pkt->hdr.src_port = 0;
> 1210 pkt->hdr.dest_port = 0;
> 1211 pkt->hdr.token = index;
> 1212 pkt->hdr.opcode = AFE_PORT_CMD_DEVICE_STOP;
>
> Uninitialized scalar variable (UNINIT)
> 5. uninit_use: Using uninitialized value port_id.
>
> 1213 stop->port_id = port_id;
> 1214 stop->reserved = 0;
>
> the commit removed the initialization of port_id = port->id, and now we
> have a regression where stop->port_id is being assigned with a garbage
> uninitialized value in port_id. Perhaps the fix needs reverting. As it
> stands, I don't know why clang was reporting this as an error.
>
> Colin
>
I suspect this patch was not made against a current tree. I sent a patch
that resolved this and Mark picked it up so it should be in the next
-next version:
https://lore.kernel.org/r/20210511190306.2418917-1-nathan@kernel.org/
Cheers,
Nathan
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <nathan@kernel.org>
To: Colin Ian King <colin.king@canonical.com>
Cc: alsa-devel@alsa-project.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Banajit Goswami <bgoswami"@codeaurora.org,
Takashi Iwai <tiwai@suse.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>,
Yang Li <yang.lee@linux.alibaba.com>
Subject: Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
Date: Wed, 12 May 2021 14:15:28 -0700 [thread overview]
Message-ID: <7b2d1562-3629-8cd3-d9a7-926c93d20e08@kernel.org> (raw)
In-Reply-To: <5935b287-5b74-84ac-3470-3b71fd69c88c@canonical.com>
On 5/12/2021 1:12 PM, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity on linux-next has detected an issue with
> the following commit:
>
> commit 5f1b95d08de712327e452d082a50fded435ec884
> Author: Yang Li <yang.lee@linux.alibaba.com>
> Date: Sun Apr 25 18:12:33 2021 +0800
>
> ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
>
> the analysis is as follows:
>
> 1181 int q6afe_port_stop(struct q6afe_port *port)
> 1182 {
> 1183 struct afe_port_cmd_device_stop *stop;
> 1184 struct q6afe *afe = port->afe;
> 1185 struct apr_pkt *pkt;
>
> 1. var_decl: Declaring variable port_id without initializer.
>
> 1186 int port_id;
> 1187 int ret = 0;
> 1188 int index, pkt_size;
> 1189 void *p;
> 1190
> 1191 index = port->token;
>
> 2. Condition index < 0, taking false branch.
> 3. Condition index >= 127, taking false branch.
>
> 1192 if (index < 0 || index >= AFE_PORT_MAX) {
> 1193 dev_err(afe->dev, "AFE port index[%d] invalid!\n",
> index);
> 1194 return -EINVAL;
> 1195 }
> 1196
> 1197 pkt_size = APR_HDR_SIZE + sizeof(*stop);
> 1198 p = kzalloc(pkt_size, GFP_KERNEL);
>
> 4. Condition !p, taking false branch.
>
> 1199 if (!p)
> 1200 return -ENOMEM;
> 1201
> 1202 pkt = p;
> 1203 stop = p + APR_HDR_SIZE;
> 1204
> 1205 pkt->hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
> 1206 APR_HDR_LEN(APR_HDR_SIZE),
> 1207 APR_PKT_VER);
> 1208 pkt->hdr.pkt_size = pkt_size;
> 1209 pkt->hdr.src_port = 0;
> 1210 pkt->hdr.dest_port = 0;
> 1211 pkt->hdr.token = index;
> 1212 pkt->hdr.opcode = AFE_PORT_CMD_DEVICE_STOP;
>
> Uninitialized scalar variable (UNINIT)
> 5. uninit_use: Using uninitialized value port_id.
>
> 1213 stop->port_id = port_id;
> 1214 stop->reserved = 0;
>
> the commit removed the initialization of port_id = port->id, and now we
> have a regression where stop->port_id is being assigned with a garbage
> uninitialized value in port_id. Perhaps the fix needs reverting. As it
> stands, I don't know why clang was reporting this as an error.
>
> Colin
>
I suspect this patch was not made against a current tree. I sent a patch
that resolved this and Mark picked it up so it should be in the next
-next version:
https://lore.kernel.org/r/20210511190306.2418917-1-nathan@kernel.org/
Cheers,
Nathan
next prev parent reply other threads:[~2021-05-12 22:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 20:12 ASoC: q6dsp: q6afe: remove unneeded dead-store initialization Colin Ian King
2021-05-12 20:12 ` Colin Ian King
2021-05-12 21:15 ` Nathan Chancellor [this message]
2021-05-12 21:15 ` Nathan Chancellor
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=7b2d1562-3629-8cd3-d9a7-926c93d20e08@kernel.org \
--to=nathan@kernel.org \
--cc="Banajit Goswami <bgoswami"@codeaurora.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=colin.king@canonical.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=srinivas.kandagatla@linaro.org \
--cc=tiwai@suse.com \
--cc=yang.lee@linux.alibaba.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.