Re: [PATCH v4] overflow: Introduce overflows_type() and castable_to_type()

Nathan Chancellor
2022-10-21T16:06:27Z

Hi Gwan-gyeong,

On Fri, Oct 21, 2022 at 11:33:33AM +0300, Gwan-gyeong Mun wrote:
> From: Kees Cook <keescook@chromium.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@gmail.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Tom Rix <trix@redhat.com>
> Cc: Daniel Latypov <dlatypov@google.com>
> Cc: Vitor Massaru Iha <vitor@massaru.org>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-hardening@vger.kernel.org
> Cc: llvm@lists.linux.dev
> Co-developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Kees Cook <keescook@chromium.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