| 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
| ||
|
Message-ID: <CAGXu5jKLHbfgZLGqxwG8LRs=0hy+iJGYQd61Ad5LghrkDWV=4w@mail.gmail.com>
Date: Mon, 10 Dec 2018 09:30:45 -0800
From: Kees Cook <keescook@...omium.org>
To: "Dmitry V. Levin" <ldv@...linux.org>
Cc: Shuah Khan <shuah@...nel.org>,
Andy Lutomirski <luto@...capital.net>,
Will Drewry <wad@...omium.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] selftests: do not macro-expand failed assertion expressions
On Sun, Dec 9, 2018 at 3:00 PM Dmitry V. Levin <ldv@...linux.org> wrote:
>
> I've stumbled over the current macro-expand behaviour of the test
> harness:
>
> $ gcc -Wall -xc - <<'__EOF__'
> TEST(macro) {
> int status = 0;
> ASSERT_TRUE(WIFSIGNALED(status));
> }
> TEST_HARNESS_MAIN
> __EOF__
> $ ./a.out
> [==========] Running 1 tests from 1 test cases.
> [ RUN ] global.macro
> <stdin>:4:global.macro:Expected 0 (0) != (((signed char) (((status) & 0x7f) + 1) >> 1) > 0) (0)
> global.macro: Test terminated by assertion
> [ FAIL ] global.macro
> [==========] 0 / 1 tests passed.
> [ FAILED ]
>
> With this change the output of the same test looks much more
> comprehensible:
>
> [==========] Running 1 tests from 1 test cases.
> [ RUN ] global.macro
> <stdin>:4:global.macro:Expected 0 (0) != WIFSIGNALED(status) (0)
> global.macro: Test terminated by assertion
> [ FAIL ] global.macro
> [==========] 0 / 1 tests passed.
> [ FAILED ]
>
> The issue is very similar to the bug fixed in glibc assert(3)
> three years ago:
> https://sourceware.org/bugzilla/show_bug.cgi?id=18604
>
> Cc: Shuah Khan <shuah@...nel.org>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Will Drewry <wad@...omium.org>
> Cc: linux-kselftest@...r.kernel.org
> Signed-off-by: Dmitry V. Levin <ldv@...linux.org>
Yeah, good idea.
Acked-by: Kees Cook <keescook@...omium.org>
-Kees
> ---
> tools/testing/selftests/kselftest_harness.h | 42 ++++++++++-----------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 6ae3730c4ee3..76d654ef3234 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -354,7 +354,7 @@
> * ASSERT_EQ(expected, measured): expected == measured
> */
> #define ASSERT_EQ(expected, seen) \
> - __EXPECT(expected, seen, ==, 1)
> + __EXPECT(expected, #expected, seen, #seen, ==, 1)
>
> /**
> * ASSERT_NE(expected, seen)
> @@ -365,7 +365,7 @@
> * ASSERT_NE(expected, measured): expected != measured
> */
> #define ASSERT_NE(expected, seen) \
> - __EXPECT(expected, seen, !=, 1)
> + __EXPECT(expected, #expected, seen, #seen, !=, 1)
>
> /**
> * ASSERT_LT(expected, seen)
> @@ -376,7 +376,7 @@
> * ASSERT_LT(expected, measured): expected < measured
> */
> #define ASSERT_LT(expected, seen) \
> - __EXPECT(expected, seen, <, 1)
> + __EXPECT(expected, #expected, seen, #seen, <, 1)
>
> /**
> * ASSERT_LE(expected, seen)
> @@ -387,7 +387,7 @@
> * ASSERT_LE(expected, measured): expected <= measured
> */
> #define ASSERT_LE(expected, seen) \
> - __EXPECT(expected, seen, <=, 1)
> + __EXPECT(expected, #expected, seen, #seen, <=, 1)
>
> /**
> * ASSERT_GT(expected, seen)
> @@ -398,7 +398,7 @@
> * ASSERT_GT(expected, measured): expected > measured
> */
> #define ASSERT_GT(expected, seen) \
> - __EXPECT(expected, seen, >, 1)
> + __EXPECT(expected, #expected, seen, #seen, >, 1)
>
> /**
> * ASSERT_GE(expected, seen)
> @@ -409,7 +409,7 @@
> * ASSERT_GE(expected, measured): expected >= measured
> */
> #define ASSERT_GE(expected, seen) \
> - __EXPECT(expected, seen, >=, 1)
> + __EXPECT(expected, #expected, seen, #seen, >=, 1)
>
> /**
> * ASSERT_NULL(seen)
> @@ -419,7 +419,7 @@
> * ASSERT_NULL(measured): NULL == measured
> */
> #define ASSERT_NULL(seen) \
> - __EXPECT(NULL, seen, ==, 1)
> + __EXPECT(NULL, "NULL", seen, #seen, ==, 1)
>
> /**
> * ASSERT_TRUE(seen)
> @@ -429,7 +429,7 @@
> * ASSERT_TRUE(measured): measured != 0
> */
> #define ASSERT_TRUE(seen) \
> - ASSERT_NE(0, seen)
> + __EXPECT(0, "0", seen, #seen, !=, 1)
>
> /**
> * ASSERT_FALSE(seen)
> @@ -439,7 +439,7 @@
> * ASSERT_FALSE(measured): measured == 0
> */
> #define ASSERT_FALSE(seen) \
> - ASSERT_EQ(0, seen)
> + __EXPECT(0, "0", seen, #seen, ==, 1)
>
> /**
> * ASSERT_STREQ(expected, seen)
> @@ -472,7 +472,7 @@
> * EXPECT_EQ(expected, measured): expected == measured
> */
> #define EXPECT_EQ(expected, seen) \
> - __EXPECT(expected, seen, ==, 0)
> + __EXPECT(expected, #expected, seen, #seen, ==, 0)
>
> /**
> * EXPECT_NE(expected, seen)
> @@ -483,7 +483,7 @@
> * EXPECT_NE(expected, measured): expected != measured
> */
> #define EXPECT_NE(expected, seen) \
> - __EXPECT(expected, seen, !=, 0)
> + __EXPECT(expected, #expected, seen, #seen, !=, 0)
>
> /**
> * EXPECT_LT(expected, seen)
> @@ -494,7 +494,7 @@
> * EXPECT_LT(expected, measured): expected < measured
> */
> #define EXPECT_LT(expected, seen) \
> - __EXPECT(expected, seen, <, 0)
> + __EXPECT(expected, #expected, seen, #seen, <, 0)
>
> /**
> * EXPECT_LE(expected, seen)
> @@ -505,7 +505,7 @@
> * EXPECT_LE(expected, measured): expected <= measured
> */
> #define EXPECT_LE(expected, seen) \
> - __EXPECT(expected, seen, <=, 0)
> + __EXPECT(expected, #expected, seen, #seen, <=, 0)
>
> /**
> * EXPECT_GT(expected, seen)
> @@ -516,7 +516,7 @@
> * EXPECT_GT(expected, measured): expected > measured
> */
> #define EXPECT_GT(expected, seen) \
> - __EXPECT(expected, seen, >, 0)
> + __EXPECT(expected, #expected, seen, #seen, >, 0)
>
> /**
> * EXPECT_GE(expected, seen)
> @@ -527,7 +527,7 @@
> * EXPECT_GE(expected, measured): expected >= measured
> */
> #define EXPECT_GE(expected, seen) \
> - __EXPECT(expected, seen, >=, 0)
> + __EXPECT(expected, #expected, seen, #seen, >=, 0)
>
> /**
> * EXPECT_NULL(seen)
> @@ -537,7 +537,7 @@
> * EXPECT_NULL(measured): NULL == measured
> */
> #define EXPECT_NULL(seen) \
> - __EXPECT(NULL, seen, ==, 0)
> + __EXPECT(NULL, "NULL", seen, #seen, ==, 0)
>
> /**
> * EXPECT_TRUE(seen)
> @@ -547,7 +547,7 @@
> * EXPECT_TRUE(measured): 0 != measured
> */
> #define EXPECT_TRUE(seen) \
> - EXPECT_NE(0, seen)
> + __EXPECT(0, "0", seen, #seen, !=, 0)
>
> /**
> * EXPECT_FALSE(seen)
> @@ -557,7 +557,7 @@
> * EXPECT_FALSE(measured): 0 == measured
> */
> #define EXPECT_FALSE(seen) \
> - EXPECT_EQ(0, seen)
> + __EXPECT(0, "0", seen, #seen, ==, 0)
>
> /**
> * EXPECT_STREQ(expected, seen)
> @@ -597,7 +597,7 @@
> if (_metadata->passed && _metadata->step < 255) \
> _metadata->step++;
>
> -#define __EXPECT(_expected, _seen, _t, _assert) do { \
> +#define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
> /* Avoid multiple evaluation of the cases */ \
> __typeof__(_expected) __exp = (_expected); \
> __typeof__(_seen) __seen = (_seen); \
> @@ -606,8 +606,8 @@
> unsigned long long __exp_print = (uintptr_t)__exp; \
> unsigned long long __seen_print = (uintptr_t)__seen; \
> __TH_LOG("Expected %s (%llu) %s %s (%llu)", \
> - #_expected, __exp_print, #_t, \
> - #_seen, __seen_print); \
> + _expected_str, __exp_print, #_t, \
> + _seen_str, __seen_print); \
> _metadata->passed = 0; \
> /* Ensure the optional handler is triggered */ \
> _metadata->trigger = 1; \
> --
> ldv
--
Kees Cook
Powered by blists - more mailing lists