linux-kernel - [PATCH v3 0/3] Add compile time sanity check of GENMASK inputs

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: <20190811184938.1796-1-rikard.falkeborn@gmail.com>
Date:   Sun, 11 Aug 2019 20:49:35 +0200
From:   Rikard Falkeborn <rikard.falkeborn@...il.com>
To:     rikard.falkeborn@...il.com
Cc:     akpm@...ux-foundation.org, joe@...ches.com,
        johannes@...solutions.net, linux-kernel@...r.kernel.org,
        yamada.masahiro@...ionext.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Kees Cook <keescook@...omium.org>, x86@...nel.org
Subject: [PATCH v3 0/3] Add compile time sanity check of GENMASK inputs

Hello,

A new attempt to try to add build time validity checks of GENMASK (and
GENMASK_ULL) inputs. There main differences from v2:

Remove a define of BUILD_BUG_ON in x86/boot to avoid a compiler warning
about redefining BUILD_BUG_ON. Instead, use the common one from
include/.

Drop patch 2 in v2 where GENMASK arguments where made more verbose.

Add a cast in the BUILD_BUG_ON_ZERO macro change the type to int to
avoid the somewhat clumpsy casts of BUILD_BUG_ON_ZERO. The second patch
in this series adds such a cast to BUILD_BUG_ON_ZERO, which makes it
possible to avoid casts when using BUILD_BUG_ON_ZERO in patch 3.

I have checked all users of BUILD_BUG_ON_ZERO and I did not find a case
where adding a cast to int would affect existing users but I'd feel much
more comfortable if someone else double (or tripple) checked (there are
~80 instances plus ~10 copies in tools). Perhaps I should have CC:d
maintainers of files using BUILD_BUG_ON_ZERO?

Finally, use __builtin_constant_p instead of __is_constexpr. This avoids
pulling in kernel.h in bits.h.

Joe Perches sent a patch series to fix existing misuses, currently there
are five such misuses (which patches pending) left in Linus tree and two
(with patches pending) in linux-next. Those patches should fix all
"simple" misuses of GENMASK (cases where the arguments are numerical
constants). Pushing v2 to linux-next also revealed an arm-specific
misuse where GENMASK was used in another macro (and also broke the
arm-builds). There is a patch to fix that by Nathan Chancellor (not in
linux-next yet). Those patches should be merged before the last patch of
this series to avoid breaking builds.

Changelog
Since v2
  - Use __builtin_constant_p instead of __is_constexpr to avoid pulling
    in kernel.h (that include was missing in v2, so the header was no
    longer builable standalone
  - add cast to BUILD_BUG_ON_ZERO to make the type int
  - Remove unnecessary casts due to the above
  - Drop patch that renamed macro arguments

Since v1
  - Add comment about why inputs are not checked when used in asm file
  - Use UL(0) instead of 0
  - Extract mask creation in a separate macro to improve readability
  - Use high and low instead of h and l (part of this was extracted to a
    separate patch)
  - Updated commit message

Rikard Falkeborn (3):
  x86/boot: Use common BUILD_BUG_ON
  linux/build_bug.h: Change type to int
  linux/bits.h: Add compile time sanity check of GENMASK inputs

 arch/x86/boot/boot.h      |  2 --
 arch/x86/boot/main.c      |  1 +
 include/linux/bits.h      | 21 +++++++++++++++++++--
 include/linux/build_bug.h |  4 ++--
 4 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.22.0