linux-kernel - Re: [PATCH][next] ALSA: firewire-lib: Fix uninitialized variable err issue

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 May 2021 14:26:24 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Colin King <colin.king@...onical.com>
Cc:     Clemens Ladisch <clemens@...isch.de>,
        Takashi Sakamoto <o-takashi@...amocchi.jp>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][next] ALSA: firewire-lib: Fix uninitialized variable err issue

On Thu, 20 May 2021 10:34:24 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@...onical.com>
> 
> Currently in the case where the payload_length is less than the
> cip_header_size the error return variable err is not being set
> and function parse_ir_ctx_header can return an uninitialized
> error return value. Fix this by setting err to zero.
> 
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: c09010eeb373 ("ALSA: firewire-lib: handle the case that empty isochronous packet payload for CIP")
> Signed-off-by: Colin Ian King <colin.king@...onical.com>

Just a bikeshed, IMO, it'd be more proper to initialize err at the
beginning than setting 0 at every branch, e.g.

--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -652,7 +652,7 @@ static int parse_ir_ctx_header(struct amdtp_stream *s, unsigned int cycle,
 	unsigned int payload_length;
 	const __be32 *cip_header;
 	unsigned int cip_header_size;
-	int err;
+	int err = 0;
 
 	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
 
@@ -683,7 +683,6 @@ static int parse_ir_ctx_header(struct amdtp_stream *s, unsigned int cycle,
 		}
 	} else {
 		cip_header = NULL;
-		err = 0;
 		*data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets;
 		*syt = 0;
 


thanks,

Takashi