linux-kernel - [PATCH v2] linux/bits.h: GENMASK_INPUT_CHECK: reduce W=2 noise by 31% treewide

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]

Date:   Tue,  8 Mar 2022 23:12:01 +0900
From:   Vincent Mailhol <mailhol.vincent@...adoo.fr>
To:     Rikard Falkeborn <rikard.falkeborn@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Emil Velikov <emil.l.velikov@...il.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        kernel test robot <lkp@...el.com>,
        Syed Nayyar Waris <syednwaris@...il.com>,
        William Breathitt Gray <vilhelm.gray@...il.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>
Subject: [PATCH v2] linux/bits.h: GENMASK_INPUT_CHECK: reduce W=2 noise by 31% treewide

This patch silences a -Wtypes-limits warning in GENMASK_INPUT_CHECK()
which is accountable for 31% of all warnings when compiling with W=2.

Indeed, GENMASK_INPUT_CHECK() will generate some warnings at W=2 level
if invoked with an unsigned integer and zero. For example, this:

| #include <linux/bits.h>
| unsigned int foo(unsigned int bar)
| { return GENMASK(bar, 0); }

would yield 30 lines of warning. Extract:

| In file included from ./include/linux/bits.h:22,
|                  from ./foo.c:1:
| foo.c: In function 'foo':
| ./include/linux/bits.h:25:36: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
|    25 |                 __is_constexpr((l) > (h)), (l) > (h), 0)))
|       |                                    ^

This pattern is harmless (false positive) and for that reason,
-Wtypes-limits was moved to W=2. c.f. [1].

However because it occurs in header files (example: find_first_bit()
from linux/find.h [2]), GENMASK_INPUT_CHECK() is accountable for
roughly 31% (164714/532484) of all W=2 warnings for an
allyesconfig. This is an issue because that noise makes it harder to
triage and find relevant W=2 warnings.

Reference (using gcc 11.2, linux v5.17-rc6 on x86_64):

| $ make allyesconfig
| $ sed -i '/CONFIG_WERROR/d' .config
| $ make W=2 -j8 2> kernel_w2.log > /dev/null
| $ grep "\./include/linux/bits\.h:.*: warning" kernel_w2\.log | wc -l
| 164714
| $ grep ": warning: " kernel_w2.log | wc -l
| 532484

In this patch, we silence this warning by:

  * replacing the comparison > by and logical and && in the first
    argument of __builtin_choose_expr().

  * casting the high bit of the mask to a signed integer in the second
    argument of __builtin_choose_expr().

[1] https://lore.kernel.org/lkml/20200708190756.16810-1-rikard.falkeborn@gmail.com/
[2] https://elixir.bootlin.com/linux/v5.17-rc6/source/include/linux/find.h#L119

Link: https://lore.kernel.org/lkml/cover.1590017578.git.syednwaris@gmail.com/
Link: https://lore.kernel.org/lkml/20220304124416.1181029-1-mailhol.vincent@wanadoo.fr/
Fixes: 295bcca84916 ("linux/bits.h: add compile time sanity check of GENMASK inputs")
Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
---
* Changelog *

v1 -> v2:

  * Rewrote the commit message to make it less verbose
  * Add in CC all people from:
  https://lore.kernel.org/lkml/cover.1590017578.git.syednwaris@gmail.com/
---
 include/linux/bits.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 87d112650dfb..542e9a8985b1 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -22,7 +22,7 @@
 #include <linux/build_bug.h>
 #define GENMASK_INPUT_CHECK(h, l) \
 	(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
-		__is_constexpr((l) > (h)), (l) > (h), 0)))
+		__is_constexpr((h)) && __is_constexpr((l)), (l) > (int)(h), 0)))
 #else
 /*
  * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
-- 
2.34.1