[PATCH 0/3] lib/test_printf: Clean up basic pointer testing

[PATCH 0/3] lib/test_printf: Clean up basic pointer testing

[Petr Mladek]

The discussion about hashing NULL pointer value[1] uncovered that
the basic tests of pointer formatting do not follow the original
structure and cause confusion.

I feel responsible for it and here is a proposed clean up.

[1] https://lkml.kernel.org/r/bcfb2f94-e7a8-0860-86e3-9fc866d98742@rasmusvillemoes.dk

Petr Mladek (3):
  lib/test_printf: Clean up test of hashed pointers
  lib/test_printf: Fix structure of basic pointer tests
  lib/test_printf: Clean up invalid pointer value testing

 lib/test_printf.c | 170 ++++++++++++++++++++----------------------------------
 1 file changed, 64 insertions(+), 106 deletions(-)

-- 
2.16.4

[PATCH 1/3] lib/test_printf: Clean up test of hashed pointers

[Petr Mladek]

The commit ad67b74d2469d9b82a ("printk: hash addresses printed with %p")
helps to prevent leaking kernel addresses.

The testing of this functionality is a bit problematic because the output
depends on a random key that is generated during boot. Though, it is
still possible to check some aspects:

  + output string length
  + hash differs from the original pointer value
  + top half bits are zeroed on 64-bit systems

This is currently done by a maze of functions:

  + It is hard to follow.
  + Some code is duplicated, e.g. the check for initialized crng.
  + The zeroed top half bits are tested only with one hardcoded PTR.
  + plain() increments "failed_tests" but not "total_tests".
  + The generic test_hashed() does not touch number of tests at all.

Move all the checks into test_hashed() so that they are done for
any given pointer that should get hashed. Also handle test counters
and internal errors the same way as the existing test() function.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/test_printf.c | 130 ++++++++++++++++++------------------------------------
 1 file changed, 42 insertions(+), 88 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 2d9f520d2f27..6fa6fb606554 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -215,29 +215,6 @@ test_string(void)
 #define PTR_VAL_NO_CRNG "(____ptrval____)"
 #define ZEROS "00000000"	/* hex 32 zero bits */
 
-static int __init
-plain_format(void)
-{
-	char buf[PLAIN_BUF_SIZE];
-	int nchars;
-
-	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
-
-	if (nchars != PTR_WIDTH)
-		return -1;
-
-	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
-		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
-			PTR_VAL_NO_CRNG);
-		return 0;
-	}
-
-	if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
-		return -1;
-
-	return 0;
-}
-
 #else
 
 #define PTR_WIDTH 8
@@ -246,88 +223,65 @@ plain_format(void)
 #define PTR_VAL_NO_CRNG "(ptrval)"
 #define ZEROS ""
 
-static int __init
-plain_format(void)
-{
-	/* Format is implicitly tested for 32 bit machines by plain_hash() */
-	return 0;
-}
-
 #endif	/* BITS_PER_LONG == 64 */
 
-static int __init
-plain_hash_to_buffer(const void *p, char *buf, size_t len)
+static void __init
+test_hashed(const char *fmt, const void *p)
 {
+	char real[PLAIN_BUF_SIZE];
+	char hash[PLAIN_BUF_SIZE];
 	int nchars;
 
-	nchars = snprintf(buf, len, "%p", p);
-
-	if (nchars != PTR_WIDTH)
-		return -1;
+	total_tests++;
 
-	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
-		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
-			PTR_VAL_NO_CRNG);
-		return 0;
+	nchars = snprintf(real, sizeof(real), "%px", p);
+	if (nchars != PTR_WIDTH) {
+		pr_err("error in test suite: vsprintf(\"%%px\", p) returned number of characters %d, expected %d\n",
+		       nchars, PTR_WIDTH);
+		goto err;
 	}
 
-	return 0;
-}
-
-static int __init
-plain_hash(void)
-{
-	char buf[PLAIN_BUF_SIZE];
-	int ret;
-
-	ret = plain_hash_to_buffer(PTR, buf, PLAIN_BUF_SIZE);
-	if (ret)
-		return ret;
-
-	if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
-		return -1;
-
-	return 0;
-}
-
-/*
- * We can't use test() to test %p because we don't know what output to expect
- * after an address is hashed.
- */
-static void __init
-plain(void)
-{
-	int err;
+	nchars = snprintf(hash, sizeof(hash), fmt, p);
+	if (nchars != PTR_WIDTH) {
+		pr_warn("vsprintf(\"%s\", p) returned number of characters %d, expected %d\n",
+			fmt, nchars, PTR_WIDTH);
+		goto err;
+	}
 
-	err = plain_hash();
-	if (err) {
-		pr_warn("plain 'p' does not appear to be hashed\n");
-		failed_tests++;
+	if (strncmp(hash, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+		pr_warn_once("crng possibly not yet initialized. vsprinf(\"%s\", p) printed \"%s\"",
+			     fmt, hash);
+		total_tests--;
 		return;
 	}
 
-	err = plain_format();
-	if (err) {
-		pr_warn("hashing plain 'p' has unexpected format\n");
-		failed_tests++;
+	/*
+	 * There is a small chance of a false negative on 32-bit systems
+	 * when the hash is the same as the pointer value.
+	 */
+	if (strncmp(hash, real, PTR_WIDTH) == 0) {
+		pr_warn("vsprintf(\"%s\", p) returned %s, expected hashed pointer\n",
+			fmt, hash);
+		goto err;
+	}
+
+#if BITS_PER_LONG == 64
+	if (strncmp(hash, ZEROS, PTR_WIDTH / 2) != 0) {
+		pr_warn("vsprintf(\"%s\", p) returned %s, expected %s in the top half bits\n",
+			fmt, hash, ZEROS);
+		goto err;
 	}
+#endif
+	return;
+
+err:
+	failed_tests++;
 }
 
 static void __init
-test_hashed(const char *fmt, const void *p)
+plain(void)
 {
-	char buf[PLAIN_BUF_SIZE];
-	int ret;
-
-	/*
-	 * No need to increase failed test counter since this is assumed
-	 * to be called after plain().
-	 */
-	ret = plain_hash_to_buffer(p, buf, PLAIN_BUF_SIZE);
-	if (ret)
-		return;
-
-	test(buf, fmt, p);
+	test_hashed("%p", PTR);
 }
 
 static void __init
-- 
2.16.4

[PATCH 2/3] lib/test_printf: Fix structure of basic pointer tests

[Petr Mladek]

The pointer formatting tests have been originally split by
the %p modifiers. For example, the function dentry() tested
%pd and %pD handling.

There were recently added tests that do not fit into
the existing structure, namely:

  + hashed pointer testing
  + null and invalid pointer handling with various modifiers

Reshuffle these tests to follow the original structure.

For completeness, add a test for "%px" with some "random" pointer
value. Note that it can't be tested with "%pE" because it would
cause crash.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/test_printf.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 6fa6fb606554..1ee1bb703307 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -278,28 +278,22 @@ test_hashed(const char *fmt, const void *p)
 	failed_tests++;
 }
 
-static void __init
-plain(void)
-{
-	test_hashed("%p", PTR);
-}
+#define PTR_INVALID ((void *)0x000000ab)
 
 static void __init
-null_pointer(void)
+plain_pointer(void)
 {
+	test_hashed("%p", PTR);
 	test_hashed("%p", NULL);
-	test(ZEROS "00000000", "%px", NULL);
-	test("(null)", "%pE", NULL);
+	test_hashed("%p", PTR_INVALID);
 }
 
-#define PTR_INVALID ((void *)0x000000ab)
-
 static void __init
-invalid_pointer(void)
+real_pointer(void)
 {
-	test_hashed("%p", PTR_INVALID);
+	test(PTR_STR, "%px", PTR);
+	test(ZEROS "00000000", "%px", NULL);
 	test(ZEROS "000000ab", "%px", PTR_INVALID);
-	test("(efault)", "%pE", PTR_INVALID);
 }
 
 static void __init
@@ -326,6 +320,8 @@ addr(void)
 static void __init
 escaped_str(void)
 {
+	test("(null)", "%pE", NULL);
+	test("(efault)", "%pE", PTR_INVALID);
 }
 
 static void __init
@@ -601,9 +597,8 @@ errptr(void)
 static void __init
 test_pointer(void)
 {
-	plain();
-	null_pointer();
-	invalid_pointer();
+	plain_pointer();
+	real_pointer();
 	symbol_ptr();
 	kernel_ptr();
 	struct_resource();
-- 
2.16.4

[PATCH 3/3] lib/test_printf: Clean up invalid pointer value testing

[Petr Mladek]

PTR_INVALID is a confusing name. It might mean any pointer value
that is not accessible. But check_pointer() function is able to
detect only the obviously invalid pointers: NULL pointer,
IS_ERR() range, and the first page.

Check all these three categories by better named values.

Use PTR_STR prefix for all expected output, including
the string used when crng has not been initialized yet.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/test_printf.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 1ee1bb703307..d640be78e3ae 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -206,13 +206,18 @@ test_string(void)
 }
 
 #define PLAIN_BUF_SIZE 64	/* leave some space so we don't oops */
+#define PTR_FIRST_PAGE ((void *)0x000000ab)
+#define PTR_STR_FIRST_PAGE "000000ab"
+#define PTR_ERROR ERR_PTR(-EFAULT)
+#define PTR_STR_ERROR "fffffff2"
 
 #if BITS_PER_LONG == 64
 
 #define PTR_WIDTH 16
 #define PTR ((void *)0xffff0123456789abUL)
 #define PTR_STR "ffff0123456789ab"
-#define PTR_VAL_NO_CRNG "(____ptrval____)"
+#define PTR_STR_NO_CRNG "(____ptrval____)"
+#define ONES "ffffffff"		/* hex 32 one bits */
 #define ZEROS "00000000"	/* hex 32 zero bits */
 
 #else
@@ -220,7 +225,8 @@ test_string(void)
 #define PTR_WIDTH 8
 #define PTR ((void *)0x456789ab)
 #define PTR_STR "456789ab"
-#define PTR_VAL_NO_CRNG "(ptrval)"
+#define PTR_STR_NO_CRNG "(ptrval)"
+#define ONES ""
 #define ZEROS ""
 
 #endif	/* BITS_PER_LONG == 64 */
@@ -248,7 +254,7 @@ test_hashed(const char *fmt, const void *p)
 		goto err;
 	}
 
-	if (strncmp(hash, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+	if (strncmp(hash, PTR_STR_NO_CRNG, PTR_WIDTH) == 0) {
 		pr_warn_once("crng possibly not yet initialized. vsprinf(\"%s\", p) printed \"%s\"",
 			     fmt, hash);
 		total_tests--;
@@ -278,22 +284,22 @@ test_hashed(const char *fmt, const void *p)
 	failed_tests++;
 }
 
-#define PTR_INVALID ((void *)0x000000ab)
-
 static void __init
 plain_pointer(void)
 {
 	test_hashed("%p", PTR);
 	test_hashed("%p", NULL);
-	test_hashed("%p", PTR_INVALID);
+	test_hashed("%p", PTR_ERROR);
+	test_hashed("%p", PTR_FIRST_PAGE);
 }
 
 static void __init
 real_pointer(void)
 {
 	test(PTR_STR, "%px", PTR);
-	test(ZEROS "00000000", "%px", NULL);
-	test(ZEROS "000000ab", "%px", PTR_INVALID);
+	test(ZEROS ZEROS, "%px", NULL);
+	test(ONES PTR_STR_ERROR, "%px", PTR_ERROR);
+	test(ZEROS PTR_STR_FIRST_PAGE, "%px", PTR_FIRST_PAGE);
 }
 
 static void __init
@@ -321,7 +327,8 @@ static void __init
 escaped_str(void)
 {
 	test("(null)", "%pE", NULL);
-	test("(efault)", "%pE", PTR_INVALID);
+	test("(efault)", "%pE", PTR_ERROR);
+	test("(efault)", "%pE", PTR_FIRST_PAGE);
 }
 
 static void __init
@@ -408,9 +415,11 @@ dentry(void)
 	test("foo", "%pd2", &test_dentry[0]);
 
 	test("(null)", "%pd", NULL);
-	test("(efault)", "%pd", PTR_INVALID);
+	test("(efault)", "%pd", PTR_ERROR);
+	test("(efault)", "%pd", PTR_FIRST_PAGE);
 	test("(null)", "%pD", NULL);
-	test("(efault)", "%pD", PTR_INVALID);
+	test("(efault)", "%pD", PTR_ERROR);
+	test("(efault)", "%pD", PTR_FIRST_PAGE);
 
 	test("romeo", "%pd", &test_dentry[3]);
 	test("alfa/romeo", "%pd2", &test_dentry[3]);
-- 
2.16.4

Re: [PATCH 1/3] lib/test_printf: Clean up test of hashed pointers

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 739 bytes --]

Hello Petr,

On 2/27/20 2:01 PM, Petr Mladek wrote:
> The commit ad67b74d2469d9b82a ("printk: hash addresses printed with %p")
> helps to prevent leaking kernel addresses.
> 
> The testing of this functionality is a bit problematic because the output
> depends on a random key that is generated during boot. Though, it is
> still possible to check some aspects:
> 
>   + output string length
>   + hash differs from the original pointer value
>   + top half bits are zeroed on 64-bit systems

Is "hash differs from the original pointer value" a valid check?
Depending on the random value and the actual pointer I can imagine a
valid match. Such a match is unlikely but not necessarily bogus, is it?

Best regards
Uwe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] lib/test_printf: Clean up test of hashed pointers

[Petr Mladek]

On Thu 2020-02-27 15:30:51, Uwe Kleine-König wrote:
> Hello Petr,
> 
> On 2/27/20 2:01 PM, Petr Mladek wrote:
> > The commit ad67b74d2469d9b82a ("printk: hash addresses printed with %p")
> > helps to prevent leaking kernel addresses.
> > 
> > The testing of this functionality is a bit problematic because the output
> > depends on a random key that is generated during boot. Though, it is
> > still possible to check some aspects:
> > 
> >   + output string length
> >   + hash differs from the original pointer value
> >   + top half bits are zeroed on 64-bit systems
> 
> Is "hash differs from the original pointer value" a valid check?
> Depending on the random value and the actual pointer I can imagine a
> valid match. Such a match is unlikely but not necessarily bogus, is it?

Yes, there is a small risk or false negative.

It might be possible to try if the problem persist with PTR+1 value or
so. But I am not sure if it is worth it.

The problem is only on 32-bit systems. The chance is really small.
I have added a comment above the check. It can be found via the added
error message.

Note that this check has been there even before in plain_hash().
But it was worse because it was without any comment or error message.

Best Regards,
Petr

Re: [PATCH 0/3] lib/test_printf: Clean up basic pointer testing

[Sergey Senozhatsky]

On (20/02/27 14:01), Petr Mladek wrote:
> 
> The discussion about hashing NULL pointer value[1] uncovered that
> the basic tests of pointer formatting do not follow the original
> structure and cause confusion.

FWIW, overall looks good to me.

> I feel responsible for it and here is a proposed clean up.

Extra points!

	-ss

Re: [PATCH 0/3] lib/test_printf: Clean up basic pointer testing

[Petr Mladek]

On Tue 2020-03-03 16:24:56, Sergey Senozhatsky wrote:
> On (20/02/27 14:01), Petr Mladek wrote:
> > 
> > The discussion about hashing NULL pointer value[1] uncovered that
> > the basic tests of pointer formatting do not follow the original
> > structure and cause confusion.
> 
> FWIW, overall looks good to me.

Could I add Acked-by or Reviewed-by tag, please?

Best Regards,
Petr

Re: [PATCH 0/3] lib/test_printf: Clean up basic pointer testing

[Sergey Senozhatsky]

On (20/03/03 10:22), Petr Mladek wrote:
> On Tue 2020-03-03 16:24:56, Sergey Senozhatsky wrote:
> > On (20/02/27 14:01), Petr Mladek wrote:
> > > 
> > > The discussion about hashing NULL pointer value[1] uncovered that
> > > the basic tests of pointer formatting do not follow the original
> > > structure and cause confusion.
> > 
> > FWIW, overall looks good to me.
> 
> Could I add Acked-by or Reviewed-by tag, please?

Sure

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

Re: [PATCH 1/3] lib/test_printf: Clean up test of hashed pointers

[Rasmus Villemoes]

On 27/02/2020 14.01, Petr Mladek wrote:
> The commit ad67b74d2469d9b82a ("printk: hash addresses printed with %p")
> helps to prevent leaking kernel addresses.
> 
> The testing of this functionality is a bit problematic because the output
> depends on a random key that is generated during boot. Though, it is
> still possible to check some aspects:
> 
>   + output string length
>   + hash differs from the original pointer value
>   + top half bits are zeroed on 64-bit systems
> 
> This is currently done by a maze of functions:
> 
>   + It is hard to follow.
>   + Some code is duplicated, e.g. the check for initialized crng.
>   + The zeroed top half bits are tested only with one hardcoded PTR.
>   + plain() increments "failed_tests" but not "total_tests".
>   + The generic test_hashed() does not touch number of tests at all.
> 
> Move all the checks into test_hashed() so that they are done for
> any given pointer that should get hashed. Also handle test counters
> and internal errors the same way as the existing test() function.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  lib/test_printf.c | 130 ++++++++++++++++++------------------------------------
>  1 file changed, 42 insertions(+), 88 deletions(-)
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 2d9f520d2f27..6fa6fb606554 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -215,29 +215,6 @@ test_string(void)
>  #define PTR_VAL_NO_CRNG "(____ptrval____)"
>  #define ZEROS "00000000"	/* hex 32 zero bits */
>  
> -static int __init
> -plain_format(void)
> -{
> -	char buf[PLAIN_BUF_SIZE];
> -	int nchars;
> -
> -	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
> -
> -	if (nchars != PTR_WIDTH)
> -		return -1;
> -
> -	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> -		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
> -			PTR_VAL_NO_CRNG);
> -		return 0;
> -	}
> -
> -	if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
> -		return -1;
> -
> -	return 0;
> -}
> -
>  #else
>  
>  #define PTR_WIDTH 8
> @@ -246,88 +223,65 @@ plain_format(void)
>  #define PTR_VAL_NO_CRNG "(ptrval)"
>  #define ZEROS ""
>  
> -static int __init
> -plain_format(void)
> -{
> -	/* Format is implicitly tested for 32 bit machines by plain_hash() */
> -	return 0;
> -}
> -
>  #endif	/* BITS_PER_LONG == 64 */
>  
> -static int __init
> -plain_hash_to_buffer(const void *p, char *buf, size_t len)
> +static void __init
> +test_hashed(const char *fmt, const void *p)
>  {
> +	char real[PLAIN_BUF_SIZE];
> +	char hash[PLAIN_BUF_SIZE];
>  	int nchars;
>  
> -	nchars = snprintf(buf, len, "%p", p);
> -
> -	if (nchars != PTR_WIDTH)
> -		return -1;
> +	total_tests++;
>  
> -	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> -		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
> -			PTR_VAL_NO_CRNG);
> -		return 0;
> +	nchars = snprintf(real, sizeof(real), "%px", p);
> +	if (nchars != PTR_WIDTH) {
> +		pr_err("error in test suite: vsprintf(\"%%px\", p) returned number of characters %d, expected %d\n",
> +		       nchars, PTR_WIDTH);
> +		goto err;
>  	}
>  
> -	return 0;
> -}
> -
> -static int __init
> -plain_hash(void)
> -{
> -	char buf[PLAIN_BUF_SIZE];
> -	int ret;
> -
> -	ret = plain_hash_to_buffer(PTR, buf, PLAIN_BUF_SIZE);
> -	if (ret)
> -		return ret;
> -
> -	if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
> -		return -1;
> -
> -	return 0;
> -}
> -
> -/*
> - * We can't use test() to test %p because we don't know what output to expect
> - * after an address is hashed.
> - */
> -static void __init
> -plain(void)
> -{
> -	int err;
> +	nchars = snprintf(hash, sizeof(hash), fmt, p);

I don't like introducing a use of snprintf in the test suite where the
compiler cannot do the basic type checking. In fact, I think we should
turn on -Werror=format (or whatever the spelling is) for test_printf.c.

So I'd much rather introduce a

int check_hashed(const char *hashed, int ret, void *p)

helper and have the caller do the "%p", p formatting to a local buffer,
pass that buffer and the snprintf return value along with the formatted
pointer p to check_hashed, then do

  failed_tests += check_hashed(...)

in the caller. Then you can use a "return 1" in the places where you now
have a "goto err".

And I think you need a rather early check in check_hashed that there's a
nul byte in the buffer that is being checked (as well as in the buffer
containing the "%px" output) before you use those buffers as %s
arguments in the error messages. do_test() carefully postpones the
comparison to the expected content (and writing of the "expected ...,
got ...") until after we at least know %s won't end up reading beyond
the end of the buffer.

> +	if (nchars != PTR_WIDTH) {
> +		pr_warn("vsprintf(\"%s\", p) returned number of characters %d, expected %d\n",
> +			fmt, nchars, PTR_WIDTH);

No, you did not call vsprintf. You called snprintf() - and vsprintf
isn't even in the call chain of that. Given that there are functions in
vsprintf.c that munge the return value (the s_c_nprintf family), please
be more precise.

> +		goto err;
> +	}
>  
> -	err = plain_hash();
> -	if (err) {
> -		pr_warn("plain 'p' does not appear to be hashed\n");
> -		failed_tests++;
> +	if (strncmp(hash, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> +		pr_warn_once("crng possibly not yet initialized. vsprinf(\"%s\", p) printed \"%s\"",
> +			     fmt, hash);
> +		total_tests--;
>  		return;
>  	}

Rather than decrementing total_tests, we should have a skipped_tests to
account for the rare case(s) where we had to skip a test for some
reason. Doing pr_warn_once for each such case is fine.

Also, typo (vsprinf), but use the right name anyway.

>  
> -	err = plain_format();
> -	if (err) {
> -		pr_warn("hashing plain 'p' has unexpected format\n");
> -		failed_tests++;
> +	/*
> +	 * There is a small chance of a false negative on 32-bit systems
> +	 * when the hash is the same as the pointer value.
> +	 */
> +	if (strncmp(hash, real, PTR_WIDTH) == 0) {
> +		pr_warn("vsprintf(\"%s\", p) returned %s, expected hashed pointer\n",
> +			fmt, hash);
> +		goto err;
> +	}
> +
> +#if BITS_PER_LONG == 64
> +	if (strncmp(hash, ZEROS, PTR_WIDTH / 2) != 0) {
> +		pr_warn("vsprintf(\"%s\", p) returned %s, expected %s in the top half bits\n",
> +			fmt, hash, ZEROS);
> +		goto err;
>  	}
> +#endif

OK, but should we also have a strspn(, "0123456789abcdef") check that
the formatted string consists of precisely PTR_WIDTH hex decimals?

Rasmus