linux-kernel - [PATCH] Squashfs: Fix use of uninitialised variable in zlib & xz decompressors

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:	Tue, 25 Jan 2011 01:33:12 +0000
From:	Phillip Lougher <phillip@...gher.demon.co.uk>
To:	Linux Kernel Development <linux-kernel@...r.kernel.org>
CC:	Jesper Juhl <jj@...osbits.net>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: [PATCH] Squashfs: Fix use of uninitialised variable in zlib & xz
 decompressors


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).

Reported-by: Jesper Juhl <jj@...osbits.net>
Signed-off-by: Phillip Lougher <phillip@...gher.demon.co.uk>
---
  fs/squashfs/block.c        |    8 ++++++++
  fs/squashfs/xz_wrapper.c   |    6 ------
  fs/squashfs/zlib_wrapper.c |    6 ------
  3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 2fb2882..8ab48bc 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -63,6 +63,14 @@ static struct buffer_head *get_block_length(struct super_block *sb,
  		*length = (unsigned char) bh->b_data[*offset] |
  			(unsigned char) bh->b_data[*offset + 1] << 8;
  		*offset += 2;
+
+		if (*offset == msblk->devblksize) {
+			put_bh(bh);
+			bh = sb_bread(sb, ++(*cur_index));
+			if (bh == NULL)
+				return NULL;
+			*offset = 0;
+		}
  	}

  	return bh;
diff --git a/fs/squashfs/xz_wrapper.c b/fs/squashfs/xz_wrapper.c
index 856756c..c4eb400 100644
--- a/fs/squashfs/xz_wrapper.c
+++ b/fs/squashfs/xz_wrapper.c
@@ -95,12 +95,6 @@ static int squashfs_xz_uncompress(struct squashfs_sb_info *msblk, void **buffer,
  			if (!buffer_uptodate(bh[k]))
  				goto release_mutex;

-			if (avail == 0) {
-				offset = 0;
-				put_bh(bh[k++]);
-				continue;
-			}
-
  			stream->buf.in = bh[k]->b_data + offset;
  			stream->buf.in_size = avail;
  			stream->buf.in_pos = 0;
diff --git a/fs/squashfs/zlib_wrapper.c b/fs/squashfs/zlib_wrapper.c
index 818a5e0..4661ae2 100644
--- a/fs/squashfs/zlib_wrapper.c
+++ b/fs/squashfs/zlib_wrapper.c
@@ -82,12 +82,6 @@ static int zlib_uncompress(struct squashfs_sb_info *msblk, void **buffer,
  			if (!buffer_uptodate(bh[k]))
  				goto release_mutex;

-			if (avail == 0) {
-				offset = 0;
-				put_bh(bh[k++]);
-				continue;
-			}
-
  			stream->next_in = bh[k]->b_data + offset;
  			stream->avail_in = avail;
  			offset = 0;
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/