From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753167Ab1AYRJV (ORCPT ); Tue, 25 Jan 2011 12:09:21 -0500 Received: from mail-fx0-f46.google.com ([209.85.161.46]:37879 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753045Ab1AYRJF convert rfc822-to-8bit (ORCPT ); Tue, 25 Jan 2011 12:09:05 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=szFhvTp6n+81HZPKGpodVh/w0kVH1dwS3s+BJ6+NCQJM5UfvXECygsUSSuxBcBSRR+ aa8S6HeLtwH9W8vdCGh9FZyeCKWltgkfgBXZlhkylQhmT9FIQKa8dRMVHEX3vhzFI0b+ FHqbWI04mXY69wuItEEONj+zcbjBJwDPJHOaU= MIME-Version: 1.0 In-Reply-To: <4D3E2858.5010000@lougher.demon.co.uk> References: <4D3E2858.5010000@lougher.demon.co.uk> Date: Tue, 25 Jan 2011 18:09:04 +0100 X-Google-Sender-Auth: o6J26ShN9PV8Kpq1sz2PNbxSZ3M Message-ID: Subject: Re: [PATCH] Squashfs: Fix use of uninitialised variable in zlib & xz decompressors From: Geert Uytterhoeven To: Phillip Lougher Cc: Linux Kernel Development , Jesper Juhl , Andrew Morton Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 25, 2011 at 02:33, Phillip Lougher wrote: > Fix potential use of uninitialised variable caused by recent decompressor > code optimisations. > > In zlib_uncompress (zlib_wrapper.c) we have > >        int zlib_err, zlib_init = 0; >        ... >        do { >                ... >                        if (avail == 0) { >                                offset = 0; >                                put_bh(bh[k++]); >                                continue; >                        } >                ... >                zlib_err = zlib_inflate(stream, Z_SYNC_FLUSH); >                ... >        } while (zlib_err == Z_OK); > > If continue is executed (avail == 0) then the while condition will be > evaluated testing zlib_err, which is uninitialised first time around the > loop. > > Fix this by getting rid of the 'if (avail == 0)' condition test, this > edge condition should not be being handled in the decompressor code, and > instead handle it generically in the caller code. > > Similarly for xz_wrapper.c. > > Incidentally, on most architectures (bar Mips and Parisc), no > uninitialised variable warning is generated by gcc, this is because > the while condition test on continue is optimised out and not performed > (when executing continue zlib_err has not been changed since entering the > loop, and logically if the while condition was true previously, then it's > still true). As this is a "do { ... } while (...);" construct and not a "while (...) { ... }" construct, the condition is not checked before doing the first iteration. Furthermore the "continue" may happen during the first iteration (this depends on parameters passed to the function), so the compiler cannot make any assumptions about the value of zlib_err, except that may be uninitialized. Gr{oetje,eeting}s,                         Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.                                 -- Linus Torvalds