* re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
@ 2021-05-12 20:12 ` Colin Ian King
0 siblings, 0 replies; 4+ messages in thread
From: Colin Ian King @ 2021-05-12 20:12 UTC (permalink / raw)
To: Yang Li
Cc: Mark Brown, Srinivas Kandagatla, Banajit Goswami <bgoswami,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel,
linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
@ 2021-05-12 20:12 ` Colin Ian King
0 siblings, 0 replies; 4+ messages in thread
From: Colin Ian King @ 2021-05-12 20:12 UTC (permalink / raw)
To: Yang Li
Cc: alsa-devel, linux-kernel, Banajit Goswami <bgoswami,
Takashi Iwai, Liam Girdwood, Mark Brown
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
2021-05-12 20:12 ` Colin Ian King
@ 2021-05-12 21:15 ` Nathan Chancellor
-1 siblings, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2021-05-12 21:15 UTC (permalink / raw)
To: Colin Ian King
Cc: Mark Brown, Srinivas Kandagatla, Banajit Goswami <bgoswami,
Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel,
linux-kernel, Yang Li
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization
@ 2021-05-12 21:15 ` Nathan Chancellor
0 siblings, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2021-05-12 21:15 UTC (permalink / raw)
To: Colin Ian King
Cc: alsa-devel, linux-kernel, Banajit Goswami <bgoswami,
Takashi Iwai, Liam Girdwood, Mark Brown, Yang Li
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-12 22:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-05-12 21:15 ` Nathan Chancellor
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.