LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	"Tobin C . Harding" <me@tobin.cc>, Joe Perches <joe@perches.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.com>
Subject: [PATCH v5 10/11] vsprintf: WARN() on invalid pointer access
Date: Wed, 25 Apr 2018 13:12:50 +0200	[thread overview]
Message-ID: <20180425111251.13246-11-pmladek@suse.com> (raw)
In-Reply-To: <20180425111251.13246-1-pmladek@suse.com>

vsprintf puts "(efault)" into the output string when it is unable
to read information from the given address.

But "(efault)" might be hard to spot. And any invalid pointer is likely
to cause problems later. It is reasonable to WARN() about it.

The only problem might be a code that rely on the fact that some
specifiers, e.g. %s, printed (null) for invalid addresses pointing
to the first or the last memory page. Such a behavior should be
avoided. But it is the reason why this change is done in a separate
patch so that it can be easily reverted.

Also we must not trigger WARN_ON() when panic_on_warn() is enabled.
Note that probe_kernel_address() was added to avoid panic() and
a potentially silent crash in printk_safe context.

Finally, we want to avoid WARN() also when testing invalid pointer access
in test_printf module. It would taint kernel and mess the kernel log.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 Documentation/core-api/printk-formats.rst | 3 +++
 lib/test_printf.c                         | 7 +++++++
 lib/vsprintf.c                            | 9 ++++++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 7c73bed2fad8..3b25adde1ec7 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -57,6 +57,9 @@ might be printed instead of the unreachable information::
 	(null)	 data on plain NULL address
 	(efault) data on invalid address
 
+Also a WARN_ON() is triggered when non-NULL address is not reachable
+and panic_on_warn is disabled.
+
 Plain Pointers
 --------------
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 45c33143fb4a..74dff6c44ec6 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -285,12 +285,19 @@ null_pointer(void)
 
 #define PTR_INVALID ((void *)0x000000ab)
 
+extern int test_printf_pointer_access;
+
 static void __init
 invalid_pointer(void)
 {
+	/* Avoid calling WARN() */
+	test_printf_pointer_access = 1;
+
 	plain(PTR_INVALID);
 	test(ZEROS "000000ab", "%px", PTR_INVALID);
 	test("(efault)", "%pE", PTR_INVALID);
+
+	test_printf_pointer_access = 0;
 }
 
 static void __init
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 5dfdc7e11d05..46e3e7c71229 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -610,6 +610,9 @@ static char *valid_string(char *buf, char *end, const char *s,
 	return widen_string(buf, len, end, spec);
 }
 
+int test_printf_pointer_access;
+EXPORT_SYMBOL_GPL(test_printf_pointer_access);
+
  /*
   * This is not a fool-proof test. 99% of the time that this will fault is
   * due to a bad pointer, not one that crosses into bad memory. Just test
@@ -623,8 +626,12 @@ static const char *check_pointer_access(const void *ptr)
 	if (!ptr)
 		return "(null)";
 
-	if (probe_kernel_address(ptr, byte))
+	/* Prevent silent crashes when called in printk_safe context. */
+	if (probe_kernel_address(ptr, byte)) {
+		WARN(!panic_on_warn && !test_printf_pointer_access,
+		     "vsprintf: invalid pointer address\n");
 		return "(efault)";
+	}
 
 	return NULL;
 }
-- 
2.13.6

  parent reply	other threads:[~2018-04-25 11:14 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 11:12 [PATCH v5 00/11] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
2018-04-25 11:12 ` [PATCH v5 01/11] vsprintf: Shuffle misc pointer to string functions Petr Mladek
2018-04-25 14:57   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 02/11] vsprintf: Add missing const ptr qualifier to prt_to_id() Petr Mladek
2018-04-25 14:57   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 03/11] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
2018-04-25 14:58   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 04/11] vsprintf: Do not check address of well-known strings Petr Mladek
2018-04-25 11:44   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 05/11] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
2018-04-25 13:08   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 06/11] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
2018-04-25 13:11   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 07/11] vsprintf: Factor out %pV handler as va_format() Petr Mladek
2018-04-25 14:56   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 08/11] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
2018-04-25 15:01   ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 09/11] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
2018-04-25 15:10   ` Andy Shevchenko
2018-04-25 15:32     ` Andy Shevchenko
2018-04-27 12:47     ` Petr Mladek
2018-05-03 11:55       ` Andy Shevchenko
2018-04-26 21:46   ` kbuild test robot
2018-04-25 11:12 ` Petr Mladek [this message]
2018-04-25 12:43   ` [PATCH v5 10/11] vsprintf: WARN() on invalid pointer access Rasmus Villemoes
2018-04-26  1:28   ` Sergey Senozhatsky
2018-04-27 12:37     ` Petr Mladek
2018-04-25 11:12 ` [PATCH v5 11/11] vsprintf: Avoid confusion between invalid address and value Petr Mladek
2018-04-27 14:10 ` [PATCH v5 00/11] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek

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=20180425111251.13246-11-pmladek@suse.com \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=me@tobin.cc \
    --cc=mhocko@suse.cz \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH v5 10/11] vsprintf: WARN() on invalid pointer access' \
    /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).