Commit 7bd57fbc authored by Ilya Dryomov's avatar Ilya Dryomov Committed by Linus Torvalds

vsprintf: don't obfuscate NULL and error pointers

I don't see what security concern is addressed by obfuscating NULL
and IS_ERR() error pointers, printed with %p/%pK.  Given the number
of sites where %p is used (over 10000) and the fact that NULL pointers
aren't uncommon, it probably wouldn't take long for an attacker to
find the hash that corresponds to 0.  Although harder, the same goes
for most common error values, such as -1, -2, -11, -14, etc.

The NULL part actually fixes a regression: NULL pointers weren't
obfuscated until commit 3e5903eb ("vsprintf: Prevent crash when
dereferencing invalid pointers") which went into 5.2.  I'm tacking
the IS_ERR() part on here because error pointers won't leak kernel
addresses and printing them as pointers shouldn't be any different
from e.g. %d with PTR_ERR_OR_ZERO().  Obfuscating them just makes
debugging based on existing pr_debug and friends excruciating.

Note that the "always print 0's for %pK when kptr_restrict == 2"
behaviour which goes way back is left as is.

Example output with the patch applied:

                             ptr         error-ptr              NULL
 %p:            0000000001f8cc5b  fffffffffffffff2  0000000000000000
 %pK, kptr = 0: 0000000001f8cc5b  fffffffffffffff2  0000000000000000
 %px:           ffff888048c04020  fffffffffffffff2  0000000000000000
 %pK, kptr = 1: ffff888048c04020  fffffffffffffff2  0000000000000000
 %pK, kptr = 2: 0000000000000000  0000000000000000  0000000000000000

Fixes: 3e5903eb ("vsprintf: Prevent crash when dereferencing invalid pointers")
Signed-off-by: default avatarIlya Dryomov <idryomov@gmail.com>
Reviewed-by: default avatarPetr Mladek <pmladek@suse.com>
Reviewed-by: default avatarSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 642b151f
...@@ -214,6 +214,7 @@ test_string(void) ...@@ -214,6 +214,7 @@ test_string(void)
#define PTR_STR "ffff0123456789ab" #define PTR_STR "ffff0123456789ab"
#define PTR_VAL_NO_CRNG "(____ptrval____)" #define PTR_VAL_NO_CRNG "(____ptrval____)"
#define ZEROS "00000000" /* hex 32 zero bits */ #define ZEROS "00000000" /* hex 32 zero bits */
#define ONES "ffffffff" /* hex 32 one bits */
static int __init static int __init
plain_format(void) plain_format(void)
...@@ -245,6 +246,7 @@ plain_format(void) ...@@ -245,6 +246,7 @@ plain_format(void)
#define PTR_STR "456789ab" #define PTR_STR "456789ab"
#define PTR_VAL_NO_CRNG "(ptrval)" #define PTR_VAL_NO_CRNG "(ptrval)"
#define ZEROS "" #define ZEROS ""
#define ONES ""
static int __init static int __init
plain_format(void) plain_format(void)
...@@ -330,14 +332,28 @@ test_hashed(const char *fmt, const void *p) ...@@ -330,14 +332,28 @@ test_hashed(const char *fmt, const void *p)
test(buf, fmt, p); test(buf, fmt, p);
} }
/*
* NULL pointers aren't hashed.
*/
static void __init static void __init
null_pointer(void) null_pointer(void)
{ {
test_hashed("%p", NULL); test(ZEROS "00000000", "%p", NULL);
test(ZEROS "00000000", "%px", NULL); test(ZEROS "00000000", "%px", NULL);
test("(null)", "%pE", NULL); test("(null)", "%pE", NULL);
} }
/*
* Error pointers aren't hashed.
*/
static void __init
error_pointer(void)
{
test(ONES "fffffff5", "%p", ERR_PTR(-11));
test(ONES "fffffff5", "%px", ERR_PTR(-11));
test("(efault)", "%pE", ERR_PTR(-11));
}
#define PTR_INVALID ((void *)0x000000ab) #define PTR_INVALID ((void *)0x000000ab)
static void __init static void __init
...@@ -649,6 +665,7 @@ test_pointer(void) ...@@ -649,6 +665,7 @@ test_pointer(void)
{ {
plain(); plain();
null_pointer(); null_pointer();
error_pointer();
invalid_pointer(); invalid_pointer();
symbol_ptr(); symbol_ptr();
kernel_ptr(); kernel_ptr();
......
...@@ -794,6 +794,13 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr, ...@@ -794,6 +794,13 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
unsigned long hashval; unsigned long hashval;
int ret; int ret;
/*
* Print the real pointer value for NULL and error pointers,
* as they are not actual addresses.
*/
if (IS_ERR_OR_NULL(ptr))
return pointer_string(buf, end, ptr, spec);
/* When debugging early boot use non-cryptographically secure hash. */ /* When debugging early boot use non-cryptographically secure hash. */
if (unlikely(debug_boot_weak_hash)) { if (unlikely(debug_boot_weak_hash)) {
hashval = hash_long((unsigned long)ptr, 32); hashval = hash_long((unsigned long)ptr, 32);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment