linux-kernel - Re: [PATCH] sbitmap: avoid maybe-uninitialized 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <940657ab-4092-cebd-127e-6628adcf96b5@kernel.dk>
Date:   Mon, 19 Sep 2016 09:22:34 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Omar Sandoval <osandov@...com>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sbitmap: avoid maybe-uninitialized warning

On 09/19/2016 09:14 AM, Arnd Bergmann wrote:
> On Monday, September 19, 2016 8:43:12 AM CEST Jens Axboe wrote:
>> On 09/19/2016 06:33 AM, Arnd Bergmann wrote:
>>> The sbitmap code that has just been turned into a library module
>>> returns uninitialized data for sbitmap_weight(), as pointed out by
>>> gcc when building with -Wmaybe-uninitialized:
>>>
>>> lib/sbitmap.c: In function 'sbitmap_weight':
>>> lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>
>>> Note that the value is never initialized, we just add data on
>>> top, so it is wrong regardless of sb->map_nr.
>>>
>>> This adds the missing initialization.
>>
>> Thanks Arnd, Colin sent the same patch and I applied it. Note that:
>
> Ok, thanks!
>
>>> Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")
>>
>> that isn't truly correct, that patch is just what moved the code. The
>> bug predates that commit.
>
> It's not important as long as the bug is fixed now, but I think it was
> correct before that move, we just lost the 'used=0' initialization,
> which also triggered the warning:
>
> -static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt)
> -{
> -       unsigned int i, used;
> -
> -       for (i = 0, used = 0; i < bt->map_nr; i++) {
> -               struct blk_align_bitmap *bm = &bt->map[i];
> -
> -               used += bitmap_weight(&bm->word, bm->depth);
>         }
>
> -       return bt->depth - used;
>  }

I missed that, as I generally never use double inits in for loops, but I
guess I did for this one. But you are right, the bug was introduced with
the recent move, so your Fixes was completely correct.

Omar loses a cookie.

-- 
Jens Axboe