Re: [PATCH] Squashfs: Fix use of uninitialised variable in zlib &xz decompressors

From: Phillip Lougher
Date: Tue Jan 25 2011 - 13:57:54 EST


Geert Uytterhoeven wrote:
On Tue, Jan 25, 2011 at 02:33, Phillip Lougher
<phillip@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/