The 64-bit overflow tests will trigger 64-bit division on 32-bit hosts,
which is not currently used anywhere in the kernel, and tickles bugs
in at least Clang 13 and earlier:
https://github.com/ClangBuiltLinux/linux/issues/1636
In reality, there shouldn't be a reason to not build the 64-bit test
cases on 32-bit systems, so these #ifdefs can be removed once the minimum
Clang version reaches 13.
In the meantime, silence W=1 warnings given by the current code:
../lib/overflow_kunit.c:191:19: warning: 's64_tests' defined but not used [-Wunused-const-variable=]
191 | DEFINE_TEST_ARRAY(s64) = {
| ^~~
../lib/overflow_kunit.c:24:11: note: in definition of macro 'DEFINE_TEST_ARRAY'
24 | } t ## _tests[]
| ^
../lib/overflow_kunit.c:94:19: warning: 'u64_tests' defined but not used [-Wunused-const-variable=]
94 | DEFINE_TEST_ARRAY(u64) = {
| ^~~
../lib/overflow_kunit.c:24:11: note: in definition of macro 'DEFINE_TEST_ARRAY'
24 | } t ## _tests[]
| ^
Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Fixes: 455a35a6cdb6 ("lib: add runtime test of check_*_overflow functions")
Cc: Rasmus Villemoes <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Vitor Massaru Iha <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Tested-by: Daniel Latypov <[email protected]>
Link: https://lore.kernel.org/lkml/CAGS_qxokQAjQRip2vPi80toW7hmBnXf=KMTNT51B1wuDqSZuVQ@mail.gmail.com
Signed-off-by: Kees Cook <[email protected]>
---
lib/overflow_kunit.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index 475f0c064bf6..7e3e43679b73 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -91,6 +91,7 @@ DEFINE_TEST_ARRAY(u32) = {
{-4U, 5U, 1U, -9U, -20U, true, false, true},
};
+#if BITS_PER_LONG == 64
DEFINE_TEST_ARRAY(u64) = {
{0, 0, 0, 0, 0, false, false, false},
{1, 1, 2, 0, 1, false, false, false},
@@ -114,6 +115,7 @@ DEFINE_TEST_ARRAY(u64) = {
false, true, false},
{-15ULL, 10ULL, -5ULL, -25ULL, -150ULL, false, false, true},
};
+#endif
DEFINE_TEST_ARRAY(s8) = {
{0, 0, 0, 0, 0, false, false, false},
@@ -188,6 +190,8 @@ DEFINE_TEST_ARRAY(s32) = {
{S32_MIN, S32_MIN, 0, 0, 0, true, false, true},
{S32_MAX, S32_MAX, -2, 0, 1, true, false, true},
};
+
+#if BITS_PER_LONG == 64
DEFINE_TEST_ARRAY(s64) = {
{0, 0, 0, 0, 0, false, false, false},
@@ -216,6 +220,7 @@ DEFINE_TEST_ARRAY(s64) = {
{-128, -1, -129, -127, 128, false, false, false},
{0, -S64_MAX, -S64_MAX, S64_MAX, 0, false, false, false},
};
+#endif
#define check_one_op(t, fmt, op, sym, a, b, r, of) do { \
t _r; \
@@ -650,6 +655,7 @@ static struct kunit_case overflow_test_cases[] = {
KUNIT_CASE(s16_overflow_test),
KUNIT_CASE(u32_overflow_test),
KUNIT_CASE(s32_overflow_test),
+/* Clang 13 and earlier generate unwanted libcalls on 32-bit. */
#if BITS_PER_LONG == 64
KUNIT_CASE(u64_overflow_test),
KUNIT_CASE(s64_overflow_test),
--
2.32.0
On Fri, Jul 01, 2022 at 05:47:10PM -0700, Kees Cook wrote:
> The 64-bit overflow tests will trigger 64-bit division on 32-bit hosts,
> which is not currently used anywhere in the kernel, and tickles bugs
> in at least Clang 13 and earlier:
> https://github.com/ClangBuiltLinux/linux/issues/1636
>
> In reality, there shouldn't be a reason to not build the 64-bit test
> cases on 32-bit systems, so these #ifdefs can be removed once the minimum
> Clang version reaches 13.
^ 14, as clang 13 has the problem too.
>
> In the meantime, silence W=1 warnings given by the current code:
>
> ../lib/overflow_kunit.c:191:19: warning: 's64_tests' defined but not used [-Wunused-const-variable=]
> 191 | DEFINE_TEST_ARRAY(s64) = {
> | ^~~
> ../lib/overflow_kunit.c:24:11: note: in definition of macro 'DEFINE_TEST_ARRAY'
> 24 | } t ## _tests[]
> | ^
> ../lib/overflow_kunit.c:94:19: warning: 'u64_tests' defined but not used [-Wunused-const-variable=]
> 94 | DEFINE_TEST_ARRAY(u64) = {
> | ^~~
> ../lib/overflow_kunit.c:24:11: note: in definition of macro 'DEFINE_TEST_ARRAY'
> 24 | } t ## _tests[]
> | ^
>
> Reported-by: kernel test robot <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Fixes: 455a35a6cdb6 ("lib: add runtime test of check_*_overflow functions")
> Cc: Rasmus Villemoes <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Vitor Massaru Iha <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Tested-by: Daniel Latypov <[email protected]>
> Link: https://lore.kernel.org/lkml/CAGS_qxokQAjQRip2vPi80toW7hmBnXf=KMTNT51B1wuDqSZuVQ@mail.gmail.com
> Signed-off-by: Kees Cook <[email protected]>
It might be nice to do something like:
/* Clang 13 and earlier generate unwanted libcalls on 32-bit. */
#if !(defined(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 140000) && BITS_PER_LONG == 32
#define EXCLUDE_64_BIT_OVERFLOW
#endif
#ifndef EXCLUDE_64_BIT_OVERFLOW
...
#endif
so that we can easily grep for CLANG_VERSION and clean this up when we
bump to a minimum version of 14.0.0 and that the scope of the workaround
is limited to the cases where it is known not to work.
However, that is ultimately up to you. Regardless:
Reviewed-by: Nathan Chancellor <[email protected]>
> ---
> lib/overflow_kunit.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
> index 475f0c064bf6..7e3e43679b73 100644
> --- a/lib/overflow_kunit.c
> +++ b/lib/overflow_kunit.c
> @@ -91,6 +91,7 @@ DEFINE_TEST_ARRAY(u32) = {
> {-4U, 5U, 1U, -9U, -20U, true, false, true},
> };
>
> +#if BITS_PER_LONG == 64
> DEFINE_TEST_ARRAY(u64) = {
> {0, 0, 0, 0, 0, false, false, false},
> {1, 1, 2, 0, 1, false, false, false},
> @@ -114,6 +115,7 @@ DEFINE_TEST_ARRAY(u64) = {
> false, true, false},
> {-15ULL, 10ULL, -5ULL, -25ULL, -150ULL, false, false, true},
> };
> +#endif
>
> DEFINE_TEST_ARRAY(s8) = {
> {0, 0, 0, 0, 0, false, false, false},
> @@ -188,6 +190,8 @@ DEFINE_TEST_ARRAY(s32) = {
> {S32_MIN, S32_MIN, 0, 0, 0, true, false, true},
> {S32_MAX, S32_MAX, -2, 0, 1, true, false, true},
> };
> +
> +#if BITS_PER_LONG == 64
> DEFINE_TEST_ARRAY(s64) = {
> {0, 0, 0, 0, 0, false, false, false},
>
> @@ -216,6 +220,7 @@ DEFINE_TEST_ARRAY(s64) = {
> {-128, -1, -129, -127, 128, false, false, false},
> {0, -S64_MAX, -S64_MAX, S64_MAX, 0, false, false, false},
> };
> +#endif
>
> #define check_one_op(t, fmt, op, sym, a, b, r, of) do { \
> t _r; \
> @@ -650,6 +655,7 @@ static struct kunit_case overflow_test_cases[] = {
> KUNIT_CASE(s16_overflow_test),
> KUNIT_CASE(u32_overflow_test),
> KUNIT_CASE(s32_overflow_test),
> +/* Clang 13 and earlier generate unwanted libcalls on 32-bit. */
> #if BITS_PER_LONG == 64
> KUNIT_CASE(u64_overflow_test),
> KUNIT_CASE(s64_overflow_test),
> --
> 2.32.0
>