linux-kernel - Re: [PATCH v4] overflow: Introduce overflows_type() and castable_to

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:   Fri, 21 Oct 2022 09:06:10 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
Cc:     keescook@...omium.org, luc.vanoostenryck@...il.com,
        ndesaulniers@...gle.com, trix@...hat.com, dlatypov@...gle.com,
        vitor@...saru.org, gustavoars@...nel.org,
        linux-hardening@...r.kernel.org, llvm@...ts.linux.dev,
        jani.nikula@...ux.intel.com, joonas.lahtinen@...ux.intel.com,
        rodrigo.vivi@...el.com, tvrtko.ursulin@...ux.intel.com,
        airlied@...ux.ie, daniel@...ll.ch, linux-kernel@...r.kernel.org,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-sparse@...r.kernel.org, arnd@...nel.org, mchehab@...nel.org,
        mauro.chehab@...ux.intel.com
Subject: Re: [PATCH v4] overflow: Introduce overflows_type() and
 castable_to_type()

Hi Gwan-gyeong,

On Fri, Oct 21, 2022 at 11:33:33AM +0300, Gwan-gyeong Mun wrote:
> From: Kees Cook <keescook@...omium.org>
> 
> Implement a robust overflows_type() macro to test if a variable or
> constant value would overflow another variable or type. This can be
> used as a constant expression for static_assert() (which requires a
> constant expression[1][2]) when used on constant values. This must be
> constructed manually, since __builtin_add_overflow() does not produce
> a constant expression[3].
> 
> Additionally adds castable_to_type(), similar to __same_type(), but for
> checking if a constant value would overflow if cast to a given type.
> 
> Add unit tests for overflows_type(), __same_type(), and castable_to_type()
> to the existing KUnit "overflow" test.
> 
> [1] https://en.cppreference.com/w/c/language/_Static_assert
> [2] C11 standard (ISO/IEC 9899:2011): 6.7.10 Static assertions
> [3] https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
>     6.56 Built-in Functions to Perform Arithmetic with Overflow Checking
>     Built-in Function: bool __builtin_add_overflow (type1 a, type2 b,
> 
> Cc: Luc Van Oostenryck <luc.vanoostenryck@...il.com>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Tom Rix <trix@...hat.com>
> Cc: Daniel Latypov <dlatypov@...gle.com>
> Cc: Vitor Massaru Iha <vitor@...saru.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> Cc: Jani Nikula <jani.nikula@...el.com>
> Cc: Mauro Carvalho Chehab <mchehab@...nel.org>
> Cc: linux-hardening@...r.kernel.org
> Cc: llvm@...ts.linux.dev
> Co-developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@...el.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>

<snip>

> diff --git a/lib/Makefile b/lib/Makefile
> index 161d6a724ff7..e061aad90539 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -376,6 +376,10 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
>  obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
>  obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
>  obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
> +# We're expecting to do a lot of "always true" or "always false" tests.
> +ifdef CONFIG_CC_IS_CLANG
> +CFLAGS_overflow_kunit.o += $(call cc-disable-warning, tautological-constant-out-of-range-compare)

If you are going to wrap this in CONFIG_CC_IS_CLANG (which is good),
drop the cc-disable-warning and just disable the warning directly.

CFLAGS_overflow_kunit.o += -Wno-tautological-constant-out-of-range-compare

All kernel supported clang versions support this warning so there is no
point in checking for its existence before disabling it with
cc-disable-warning. scripts/Makefile.extrawarn does this as well.

> +endif
>  obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
>  CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
>  obj-$(CONFIG_ST&ACKINIT_KUNIT_TEST) += stackinit_kunit.o

Cheers,
Nathan