Message-ID: <20190228085641.GA7991@archlinux-ryzen>
Date:   Thu, 28 Feb 2019 01:56:41 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Qian Cai <cai@....pw>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        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>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

On Wed, Feb 27, 2019 at 09:09:40AM -0500, Qian Cai wrote:
> 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

Unfortunately, scripts/Kbuild.extrawarn disables -Wuninitialized for
Clang, which also disables -Wsometimes-uninitialized:

https://github.com/ClangBuiltLinux/linux/issues/381
https://clang.llvm.org/docs/DiagnosticsReference.html#wuninitialized

I'm going to be sending out patches to fix the warnings found with it
then enable it going forward so that things like this get caught.

Nathan

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

Powered by blists - more mailing lists