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-next>] [day] [month] [year] [list]
Date:   Wed, 12 May 2021 21:12:37 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Yang Li <yang.lee@...ux.alibaba.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>
Subject: re: ASoC: q6dsp: q6afe: remove unneeded dead-store initialization

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ