linux-kernel - Re: [GIT PULL] Cgroup writeback support for 4.2

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: <CAMuHMdWg-Nosb4pk=uB_hS=cZHgKirV-MmCXh+PBd=e=82if8g@mail.gmail.com>
Date:	Fri, 26 Jun 2015 15:57:18 +0200
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Tejun Heo <tj@...nel.org>
Cc:	Jens Axboe <axboe@...com>, torvalds@...uxfoundation.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] Cgroup writeback support for 4.2

Hi Tejun,

On Fri, Jun 26, 2015 at 3:43 PM, Tejun Heo <tj@...nel.org> wrote:
> On Fri, Jun 26, 2015 at 11:49:58AM +0200, Geert Uytterhoeven wrote:
>> Spoiler for TLDR: These are all false positives.
>>
>> If CONFIG_CGROUP_WRITEBACK=n:
>>
>>     mm/page-writeback.c: In function ‘balance_dirty_pages_ratelimited’:
>>     mm/page-writeback.c:1574: warning: ‘writeback’ is used
>> uninitialized in this function
>>
>> In this case, mem_cgroup_wb_stats() is a dummy function that doesn't
>> write to its output parameters, hence writeback will contain arbitrary data.
>>
>> There's another call too mem_cgroup_wb_stats() in
>> wb_over_bg_thresh() where my gcc 4.1.2 didn't warn, where it probably
>> deduced that mdtc will always be NULL, and the branch thus never taken.
>
> Can you please tell me the version of gcc which triggered the above
> warnings?

gcc 4.1.2

>> Regardless of CONFIG_CGROUP_WRITEBACK:
>>
>>     mm/page-writeback.c: In function ‘balance_dirty_pages_ratelimited’:
>>     mm/page-writeback.c:1537: warning: ‘m_bg_thresh’ may be used
>> uninitialized in this function
>>     mm/page-writeback.c:1537: warning: ‘m_thresh’ may be used
>> uninitialized in this function
>>     mm/page-writeback.c:1537: warning: ‘m_dirty’ may be used
>> uninitialized in this function
>>
>> But these are false positives too, due to the many tests on mdtc or !mdtc,
>> and the creative use of dummy *_INIT() macros and mdtc_valid() static inline
>> functions.
>> I suggest refactoring this code to make it less fragile, though.
>
> It's written that way deliberately with the purpose of triggering
> compile warnings if later code breaks something and code paths which
> should be left out when !CGROUP_WRITEBACK aren't while not impeding
> readability with ifdefs and awakwardly split dummy functions.
>
> The hope was that most compilers in use today are smart enough to
> notice the code paths which are being disabled (it's pretty darn
> obvious) and it seemed that way given the dearth of build warning
> reports from -next.

That's why I keep on using gcc 4.1.2: it still gives build warnings for
many "used uninitialized" cases that later gcc versions let pass silently.
Granted, some of these are false positives (that's why it was disabled in
later gcc versions), but some of these are valid and real bugs.

Anyway, as a casual reader, it took me a while to notice all four warnings
listed above are false positives...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/