linux-kernel - Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 12 May 2021 14:15:28 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Colin Ian King <colin.king@...onical.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        "Banajit Goswami <bgoswami"@codeaurora.org,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Yang Li <yang.lee@...ux.alibaba.com>
Subject: Re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization

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@...ux.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