linux-kernel - Re: [PATCH] waitqueue: fix clang -Wuninitialized warnings

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190711174949.dc74310efd1fd3c8bd4ea276@linux-foundation.org>
Date:   Thu, 11 Jul 2019 17:49:49 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com,
        Nathan Chancellor <natechancellor@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH] waitqueue: fix clang -Wuninitialized warnings

On Wed,  3 Jul 2019 10:10:55 +0200 Arnd Bergmann <arnd@...db.de> wrote:

> When CONFIG_LOCKDEP is set, every use of DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
> produces an annoying warning from clang, which is particularly annoying
> for allmodconfig builds:
> 
> fs/namei.c:1646:34: error: variable 'wq' is uninitialized when used within its own initialization [-Werror,-Wuninitialized]
>         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>                                         ^~
> include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
>         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
>                                ~~~~                                  ^~~~
> include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
>         ({ init_waitqueue_head(&name); name; })
>                                        ^~~~

<scratches head>

Surely clang is being extraordinarily dumb here?

DECLARE_WAIT_QUEUE_HEAD_ONSTACK() is effectively doing

	struct wait_queue_head name = ({ __init_waitqueue_head(&name) ; name; })

which is perfectly legitimate!  clang has no business assuming that
__init_waitqueue_head() will do any reads from the pointer which it was
passed, nor can clang assume that __init_waitqueue_head() leaves any of
*name uninitialized.

Does it also warn if code does this?

	struct wait_queue_head name;
	__init_waitqueue_head(&name);
	name = name;

which is equivalent, isn't it?

The proposed solution is, effectively, to open-code
__init_waitqueue_head() at each DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
callsite.  That's pretty unpleasant and calls for an explanatory
comment at the __WAIT_QUEUE_HEAD_INIT_ONSTACK() definition site as well
as a cautionary comment at the __init_waitqueue_head() definition so we
can keep the two versions in sync as code evolves.

Hopefully clang will soon be hit with the cluebat (yes?) and this
change becomes obsolete in the quite short term.  Surely 6-12 months
from now nobody will be using the uncluebatted version of clang on
contemporary kernel sources so we get to remove this nastiness again. 
Which makes me wonder whether we should merge it at all.