linux-kernel - Re: [PATCH] rbd: work around -Wuninitialized warning

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] [day] [month] [year] [list]

Message-ID: <CAK8P3a0qb3fE1D4o1wYMVVets8CtbTpMRg1hUJF1wW+oC1GJjg@mail.gmail.com>
Date:   Wed, 8 Jan 2020 17:05:25 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Ilya Dryomov <idryomov@...il.com>
Cc:     Sage Weil <sage@...hat.com>, Jens Axboe <axboe@...nel.dk>,
        Oleksandr Natalenko <oleksandr@...hat.com>,
        Dongsheng Yang <dongsheng.yang@...ystack.cn>,
        Jason Dillaman <dillaman@...hat.com>,
        David Howells <dhowells@...hat.com>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rbd: work around -Wuninitialized warning

On Wed, Jan 8, 2020 at 4:31 PM Ilya Dryomov <idryomov@...il.com> wrote:
>
> On Tue, Jan 7, 2020 at 10:02 PM Arnd Bergmann <arnd@...db.de> wrote:
> >
> > gcc -O3 warns about a dummy variable that is passed
> > down into rbd_img_fill_nodata without being initialized:
> >
> > drivers/block/rbd.c: In function 'rbd_img_fill_nodata':
> > drivers/block/rbd.c:2573:13: error: 'dummy' is used uninitialized in this function [-Werror=uninitialized]
> >   fctx->iter = *fctx->pos;
> >
> > Since this is a dummy, I assume the warning is harmless, but
> > it's better to initialize it anyway and avoid the warning.
> >
> > Fixes: mmtom ("init/Kconfig: enable -O3 for all arches")
> > Signed-off-by: Arnd Bergmann <arnd@...db.de>
> > ---
> >  drivers/block/rbd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index 29be02838b67..070edc5983df 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -2664,7 +2664,7 @@ static int rbd_img_fill_nodata(struct rbd_img_request *img_req,
> >                                u64 off, u64 len)
> >  {
> >         struct ceph_file_extent ex = { off, len };
> > -       union rbd_img_fill_iter dummy;
> > +       union rbd_img_fill_iter dummy = {};
> >         struct rbd_img_fill_ctx fctx = {
> >                 .pos_type = OBJ_REQUEST_NODATA,
> >                 .pos = &dummy,
>
> Applied, but slightly confused.  Wasn't selecting -O3/s/etc supposed to
> automatically disable -Wmaybe-uninitialized via Kconfig?

Oh, that's right. I have a couple of patches in my randconfig tree that
completely rework the way that the warning options are handled and
that accidentally ignored CONFIG_CC_DISABLE_WARN_MAYBE_UNINITIALIZED,
so it's won't actually happen on linux-next right now, just on my kernel.

However, given that -O3 did not actually introduce too many false
positives here but did find some actual uninitialized variables, we should
probably have it turned on anyway.

A lot of these false positives seem to happen whenever gcc can partially
understand how a variable is used, but not enough to see that it's ok.
With higher optimization levels, this happens less often than with the
lower levels as it inlines more aggressively and correctly determines
uses to be safe that were false-positives earlier.

I'm fairly sure that the output at -Os still won't be helpful as that would
mostly show up cases that -O2 has found to be safe rather than those
that -O2 decided not to warn about because of lack of information.

      Arnd