linux-kernel - [PATCH kselftest/test] kunit: Always print actual pointer values in asserts

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-next>] [day] [month] [year] [list]

Message-Id: <20191121235058.21653-1-davidgow@google.com>
Date:   Thu, 21 Nov 2019 15:50:58 -0800
From:   David Gow <davidgow@...gle.com>
To:     Brendan Higgins <brendanhiggins@...gle.com>, shuah@...nel.org
Cc:     linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
        linux-kernel@...r.kernel.org, David Gow <davidgow@...gle.com>
Subject: [PATCH kselftest/test] kunit: Always print actual pointer values in asserts

KUnit assertions and expectations will print the values being tested. If
these are pointers (e.g., KUNIT_EXPECT_PTR_EQ(test, a, b)), these
pointers are currently printed with the %pK format specifier, which -- to
prevent information leaks which may compromise, e.g., ASLR -- are often
either hashed or replaced with ____ptrval____ or similar, making debugging
tests difficult.

By replacing %pK with %px as Documentation/core-api/printk-formats.rst
suggests, we disable this security feature for KUnit assertions and
expectations, allowing the actual pointer values to be printed. Given
that KUnit is not intended for use in production kernels, and the
pointers are only printed on failing tests, this seems like a worthwhile
tradeoff.

Signed-off-by: David Gow <davidgow@...gle.com>
---
This seems like the best way of solving this problem to me, but if
anyone has a better solution I'd love to hear it.

Note also that this does trigger two checkpatch.pl warnings, which warn
that the change will potentially cause the kernel memory layout to be
exposed. Since that's the whole point of the change, they probably
sohuld stay there.

 lib/kunit/assert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index 86013d4cf891..a87960409bd4 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -110,10 +110,10 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert,
 			 binary_assert->left_text,
 			 binary_assert->operation,
 			 binary_assert->right_text);
-	string_stream_add(stream, "\t\t%s == %pK\n",
+	string_stream_add(stream, "\t\t%s == %px\n",
 			 binary_assert->left_text,
 			 binary_assert->left_value);
-	string_stream_add(stream, "\t\t%s == %pK",
+	string_stream_add(stream, "\t\t%s == %px",
 			 binary_assert->right_text,
 			 binary_assert->right_value);
 	kunit_assert_print_msg(assert, stream);
-- 
2.24.0.432.g9d3f5f5b63-goog