linux-kernel - Re: [PATCH] tmpfs: fix uninitialized return value in shmem

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: <1551276580.7087.1.camel@lca.pw>
Date:   Wed, 27 Feb 2019 09:09:40 -0500
From:   Qian Cai <cai@....pw>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Hugh Dickins <hughd@...gle.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Matej Kupljen <matej.kupljen@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

On Mon, 2019-02-25 at 16:07 -0800, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 4:03 PM Qian Cai <cai@....pw> wrote:
> > > 
> > > Of course, that's just gcc. I have no idea what llvm ends up doing.
> > 
> > Clang 7.0:
> > 
> > # clang  -O2 -S -Wall /tmp/test.c
> > /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever
> > 'if'
> > condition is false [-Wsometimes-uninitialized]
> 
> Ok, good.
> 
> Do we have any clang builds in any of the zero-day robot
> infrastructure or something? Should we?
> 
> And maybe this was how Dan noticed the problem in the first place? Or
> is it just because of his eagle-eyes?
> 

BTW, even clang is able to generate warnings in your sample code, it does not
generate any warnings when compiling the buggy shmem.o via "make CC=clang". Here
is the objdump for arm64 (with KASAN_SW_TAGS inline).

000000000000effc <shmem_link>:
{
    effc:       f81c0ff7        str     x23, [sp, #-64]!
    f000:       a90157f6        stp     x22, x21, [sp, #16]
    f004:       a9024ff4        stp     x20, x19, [sp, #32]
    f008:       a9037bfd        stp     x29, x30, [sp, #48]
    f00c:       9100c3fd        add     x29, sp, #0x30
    f010:       aa0203f3        mov     x19, x2
    f014:       aa0103f5        mov     x21, x1
    f018:       aa0003f4        mov     x20, x0
    f01c:       94000000        bl      0 <_mcount>
    f020:       91016280        add     x0, x20, #0x58
    f024:       d2c20017        mov     x23, #0x100000000000            //
#17592186044416
    f028:       b2481c08        orr     x8, x0, #0xff00000000000000
    f02c:       f2fdfff7        movk    x23, #0xefff, lsl #48
    f030:       d344fd08        lsr     x8, x8, #4
    f034:       38776909        ldrb    w9, [x8, x23]
    f038:       940017d5        bl      14f8c <OUTLINED_FUNCTION_11>
    f03c:       54000060        b.eq    f048 <shmem_link+0x4c>  // b.none
    f040:       7103fd1f        cmp     w8, #0xff
    f044:       54000981        b.ne    f174 <shmem_link+0x178>  // b.any
    f048:       f9400014        ldr     x20, [x0]
        if (inode->i_nlink) {
    f04c:       91010280        add     x0, x20, #0x40
    f050:       b2481c08        orr     x8, x0, #0xff00000000000000
    f054:       d344fd08        lsr     x8, x8, #4
    f058:       38776909        ldrb    w9, [x8, x23]
    f05c:       940017cc        bl      14f8c <OUTLINED_FUNCTION_11>
    f060:       54000060        b.eq    f06c <shmem_link+0x70>  // b.none
    f064:       7103fd1f        cmp     w8, #0xff
    f068:       540008a1        b.ne    f17c <shmem_link+0x180>  // b.any
    f06c:       b9400008        ldr     w8, [x0]
    f070:       34000148        cbz     w8, f098 <shmem_link+0x9c>
    f074:       940018fd        bl      15468 <OUTLINED_FUNCTION_1124>
                ret = shmem_reserve_inode(inode->i_sb);
    f078:       38776909        ldrb    w9, [x8, x23]
    f07c:       940017c4        bl      14f8c <OUTLINED_FUNCTION_11>
    f080:       54000060        b.eq    f08c <shmem_link+0x90>  // b.none
    f084:       7103fd1f        cmp     w8, #0xff
    f088:       540007e1        b.ne    f184 <shmem_link+0x188>  // b.any
    f08c:       f9400000        ldr     x0, [x0]
    f090:       97fffcf6        bl      e468 <shmem_reserve_inode>
                if (ret)
    f094:       35000660        cbnz    w0, f160 <shmem_link+0x164>
        dir->i_size += BOGO_DIRENT_SIZE;
    f098:       910122a0        add     x0, x21, #0x48
    f09c:       b2481c08        orr     x8, x0, #0xff00000000000000
    f0a0:       d344fd09        lsr     x9, x8, #4
    f0a4:       3877692a        ldrb    w10, [x9, x23]
    f0a8:       94001828        bl      15148 <OUTLINED_FUNCTION_193>
    f0ac:       54000060        b.eq    f0b8 <shmem_link+0xbc>  // b.none
    f0b0:       7103fd1f        cmp     w8, #0xff
    f0b4:       540006c1        b.ne    f18c <shmem_link+0x190>  // b.any
    f0b8:       38776929        ldrb    w9, [x9, x23]
    f0bc:       94001a4a        bl      159e4 <OUTLINED_FUNCTION_1131>
    f0c0:       54000060        b.eq    f0cc <shmem_link+0xd0>  // b.none
    f0c4:       7103fd1f        cmp     w8, #0xff
    f0c8:       54000661        b.ne    f194 <shmem_link+0x198>  // b.any
    f0cc:       f9000009        str     x9, [x0]
        inode->i_ctime = dir->i_ctime = dir->i_mtime = current_time(inode);
    f0d0:       aa1403e0        mov     x0, x20
    f0d4:       910182b6        add     x22, x21, #0x60
    f0d8:       94000000        bl      0 <current_time>
    f0dc:       b2481ec9        orr     x9, x22, #0xff00000000000000
    f0e0:       d344fd29        lsr     x9, x9, #4