Skip to content

Commit 7bd57fb

Browse files
idryomovtorvalds
authored andcommitted
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 3e5903e ("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: 3e5903e ("vsprintf: Prevent crash when dereferencing invalid pointers") Signed-off-by: Ilya Dryomov <[email protected]> Reviewed-by: Petr Mladek <[email protected]> Reviewed-by: Sergey Senozhatsky <[email protected]> Reviewed-by: Andy Shevchenko <[email protected]> Acked-by: Steven Rostedt (VMware) <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 642b151 commit 7bd57fb

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

lib/test_printf.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ test_string(void)
214214
#define PTR_STR "ffff0123456789ab"
215215
#define PTR_VAL_NO_CRNG "(____ptrval____)"
216216
#define ZEROS "00000000" /* hex 32 zero bits */
217+
#define ONES "ffffffff" /* hex 32 one bits */
217218

218219
static int __init
219220
plain_format(void)
@@ -245,6 +246,7 @@ plain_format(void)
245246
#define PTR_STR "456789ab"
246247
#define PTR_VAL_NO_CRNG "(ptrval)"
247248
#define ZEROS ""
249+
#define ONES ""
248250

249251
static int __init
250252
plain_format(void)
@@ -330,14 +332,28 @@ test_hashed(const char *fmt, const void *p)
330332
test(buf, fmt, p);
331333
}
332334

335+
/*
336+
* NULL pointers aren't hashed.
337+
*/
333338
static void __init
334339
null_pointer(void)
335340
{
336-
test_hashed("%p", NULL);
341+
test(ZEROS "00000000", "%p", NULL);
337342
test(ZEROS "00000000", "%px", NULL);
338343
test("(null)", "%pE", NULL);
339344
}
340345

346+
/*
347+
* Error pointers aren't hashed.
348+
*/
349+
static void __init
350+
error_pointer(void)
351+
{
352+
test(ONES "fffffff5", "%p", ERR_PTR(-11));
353+
test(ONES "fffffff5", "%px", ERR_PTR(-11));
354+
test("(efault)", "%pE", ERR_PTR(-11));
355+
}
356+
341357
#define PTR_INVALID ((void *)0x000000ab)
342358

343359
static void __init
@@ -649,6 +665,7 @@ test_pointer(void)
649665
{
650666
plain();
651667
null_pointer();
668+
error_pointer();
652669
invalid_pointer();
653670
symbol_ptr();
654671
kernel_ptr();

lib/vsprintf.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,13 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
794794
unsigned long hashval;
795795
int ret;
796796

797+
/*
798+
* Print the real pointer value for NULL and error pointers,
799+
* as they are not actual addresses.
800+
*/
801+
if (IS_ERR_OR_NULL(ptr))
802+
return pointer_string(buf, end, ptr, spec);
803+
797804
/* When debugging early boot use non-cryptographically secure hash. */
798805
if (unlikely(debug_boot_weak_hash)) {
799806
hashval = hash_long((unsigned long)ptr, 32);

0 commit comments

Comments
 (0)