LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ilya Dryomov <idryomov@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Kees Cook <keescook@chromium.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	"Tobin C . Harding" <me@tobin.cc>
Subject: [PATCH] vsprintf: don't obfuscate NULL and error pointers
Date: Mon, 17 Feb 2020 23:28:03 +0100	[thread overview]
Message-ID: <20200217222803.6723-1-idryomov@gmail.com> (raw)

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 3e5903eb9cff ("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: 3e5903eb9cff ("vsprintf: Prevent crash when dereferencing invalid pointers")
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 lib/vsprintf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7c488a1ce318..f0f0522cd5a7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -794,6 +794,13 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
 	unsigned long hashval;
 	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. */
 	if (unlikely(debug_boot_weak_hash)) {
 		hashval = hash_long((unsigned long)ptr, 32);
-- 
2.19.2


             reply	other threads:[~2020-02-17 22:27 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-17 22:28 Ilya Dryomov [this message]
2020-02-17 23:47 ` Kees Cook
2020-02-18  0:07   ` Ilya Dryomov
2020-02-18 10:33     ` Petr Mladek
2020-02-18 11:16       ` Ilya Dryomov
2020-02-18 16:50       ` Steven Rostedt
2020-02-19  2:13       ` Sergey Senozhatsky
2020-02-18 18:49     ` Linus Torvalds
2020-02-18 19:31       ` Adam Borowski
2020-02-18 19:38         ` Linus Torvalds
2020-02-18 20:19           ` Ilya Dryomov
2020-02-18 20:21             ` Linus Torvalds
2020-02-19  7:30             ` Rasmus Villemoes
2020-02-19  8:21           ` [PATCH] vsprintf: sanely handle NULL passed to %pe Rasmus Villemoes
2020-02-19  9:35             ` Andy Shevchenko
2020-02-19 11:20             ` Ilya Dryomov
2020-02-19 11:25               ` Andy Shevchenko
2020-02-19 11:29                 ` Ilya Dryomov
2020-02-19 11:53               ` Rasmus Villemoes
2020-02-19 13:48                 ` Petr Mladek
2020-02-19 13:56                   ` Rasmus Villemoes
2020-02-19 14:45                     ` Petr Mladek
2020-02-19 15:38                       ` Andy Shevchenko
2020-02-19 15:40                       ` Rasmus Villemoes
2020-02-19 17:23                         ` Ilya Dryomov
2020-02-20 12:57                         ` Petr Mladek
2020-02-20 15:02                           ` Ilya Dryomov
2020-02-21 13:05                             ` Petr Mladek
2020-02-21 23:52                               ` Rasmus Villemoes
2020-02-22  8:14                                 ` Andy Shevchenko
2020-02-24  9:55                                 ` Petr Mladek
2020-02-18 18:44 ` [PATCH] vsprintf: don't obfuscate NULL and error pointers Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200217222803.6723-1-idryomov@gmail.com \
    --to=idryomov@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@tobin.cc \
    --cc=pmladek@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH] vsprintf: don'\''t obfuscate NULL and error pointers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).