linux-kernel - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D3F1D2A.9040309@lougher.demon.co.uk>
Date:	Tue, 25 Jan 2011 18:57:46 +0000
From:	Phillip Lougher <phillip@...gher.demon.co.uk>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
CC:	Linux Kernel Development <linux-kernel@...r.kernel.org>,
	Jesper Juhl <jj@...osbits.net>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Squashfs: Fix use of uninitialised variable in zlib &
 xz decompressors

Geert Uytterhoeven wrote:
> On Tue, Jan 25, 2011 at 02:33, Phillip Lougher
> <phillip@...gher.demon.co.uk> wrote:
>> 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.
> 

No, I've checked the assembly language produced - the while condition test has
been optimised out in the case of continue.

This is the assembly language output for x86_64, the relevant
bits manually annotated by me,

zlib_uncompress:
.LFB1375:
	...
	call	mutex_lock_nested
	movl	$0, 32(%r12)
	movl	$0, 8(%r12)
	xorl	%edx, %edx
	xorl	%eax, %eax

# jump to top of loop

	jmp	.L12
	.p2align 4,,10
	.p2align 3

.L25:

# whole if (stream->avail_in == 0 && k < b) { block of code moved out of
# main loop, so continue can fall through to main loop

#  if (stream->avail_in == 0)

	testl	%edx, %edx
	jne	.L2

# wait_on_buffer(bh[k]);

	movq	32(%rsp), %rsi
	movq	48(%rsp), %rdx
	movslq	%r13d,%rcx
	movq	%rcx, 56(%rsp)
	movl	$.LC0, %edi
	movl	%eax, (%rsp)
	leaq	(%rsi,%rcx,8), %rcx
	movl	8(%rdx), %edx
	movl	$306, %esi
	movq	(%rcx), %r8
	movq	%rcx, 8(%rsp)
	movl	%edx, 68(%rsp)
	xorl	%edx, %edx
	movq	%r8, 16(%rsp)
	call	__might_sleep
	movq	16(%rsp), %r8
	movl	(%rsp), %eax
	movq	8(%rsp), %rcx
	movq	(%r8), %rdx
	andl	$4, %edx
	jne	.L24
.L3:

# if (!buffer_uptodate(bh[k]))
#	goto release_mutex;

	movq	(%rcx), %rdx
	movq	(%rdx), %rcx
	andl	$1, %ecx
	je	.L4

# int avail = min(length, msblk->devblksize - offset);

	movl	68(%rsp), %ecx
	subl	44(%rsp), %ecx
	cmpl	28(%rsp), %ecx
	cmovg	28(%rsp), %ecx

# length -= avail;

	subl	%ecx, 28(%rsp)

# if (avail == 0)

	testl	%ecx, %ecx
	jne	.L5

# put_bh(bh[k++]);

	addl	$1, %r13d
#APP
# 107 "/stripe/git-trees/linux-linus-bugfix-1/arch/x86/include/asm/atomic.h" 1
	.section .smp_locks,"a"
.balign 4
.long 671f - .
.previous
671:
	lock; decl 96(%rdx)
# 0 "" 2
#NO_APP

# offset = 0;

	movl	$0, 44(%rsp)
	.p2align 4,,10
	.p2align 3

# *continue* fails through to top of loop
# no while condition test


# top of loop

.L6:
	movl	8(%r12), %edx

# optimised  case - first iteration around loop enters here as
# edx doesn't need to be loaded (it already holds 0 from
# stream->avail_in = 0)

.L12:

# if (k < b)

	cmpl	%ebp, %r13d
	setl	%r15b
	jl	.L25
.L2:

# if (stream->avail_out == 0 && page < pages) {

	cmpl	152(%rsp), %r14d
	jge	.L7
	movl	32(%r12), %ecx
	testl	%ecx, %ecx
	jne	.L7
	movslq	%r14d,%rdx
	movl	$4096, 32(%r12)
	addl	$1, %r14d
	movq	(%rbx,%rdx,8), %rdx
	movq	%rdx, 24(%r12)
.L7:

# if (!zlib_init) {

	testl	%eax, %eax
	jne	.L8
	movl	$15, %esi
	movq	%r12, %rdi
	call	zlib_inflateInit2
	testl	%eax, %eax
	jne	.L26
.L8:

# zlib_err = zlib_inflate(stream, Z_SYNC_FLUSH);

	movl	$3, %esi
	movq	%r12, %rdi
	call	zlib_inflate

# if( stream->avail_in == 0 && k < b) {

	testb	%r15b, %r15b
	je	.L10
	movl	8(%r12), %edx
	testl	%edx, %edx
	jne	.L10
	movq	32(%rsp), %rcx
	movslq	%r13d,%rdx
	addl	$1, %r13d
	movq	(%rcx,%rdx,8), %rdx
#APP
# 107 "/stripe/git-trees/linux-linus-bugfix-1/arch/x86/include/asm/atomic.h" 1
	.section .smp_locks,"a"
.balign 4
.long 671f - .
.previous
671:
	lock; decl 96(%rdx)
# 0 "" 2
#NO_APP


.L10:

# } while (zlib_err == Z_OK);

	testl	%eax, %eax
	jne	.L11
	movb	$1, %al

# jump to top of loop

	jmp	.L6
	.p2align 4,,10
	.p2align 3

Phillip
--
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/