All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"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 v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers
Date: Wed,  4 Apr 2018 10:58:38 +0200	[thread overview]
Message-ID: <20180404085843.16050-5-pmladek@suse.com> (raw)
In-Reply-To: <20180404085843.16050-1-pmladek@suse.com>

There are few printk formats that make sense only with two or more
specifiers. Also some specifiers make sense only when a kernel feature
is enabled.

The handling of unknown specifiers is strange, inconsistent, and
even leaking the address. For example, netdev_bits() prints the
non-hashed pointer value or clock() prints "(null)".

The best solution seems to be in flags_string(). It does not print any
misleading value. Instead it calls WARN_ONCE() describing the unknown
specifier. Therefore it clearly shows the problem and helps to find it.

Note that WARN_ONCE() used to cause recursive printk(). But it is safe
now because vscnprintf() is called in printk_safe context from
vprintk_emit().

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/vsprintf.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index dd71738d7a09..5a0d01846a11 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1484,9 +1484,9 @@ char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt)
 		size = sizeof(netdev_features_t);
 		break;
 	default:
-		num = (unsigned long)addr;
-		size = sizeof(unsigned long);
-		break;
+		WARN_ONCE(1, "Unsupported pointer format specifier: %%pN%c\n",
+			  fmt[1]);
+		return buf;
 	}
 
 	return special_hex_number(buf, end, num, size);
@@ -1517,7 +1517,12 @@ static noinline_for_stack
 char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 	    const char *fmt)
 {
-	if (!IS_ENABLED(CONFIG_HAVE_CLK) || !clk)
+	if (!IS_ENABLED(CONFIG_HAVE_CLK)) {
+		WARN_ONCE(1, "Unsupported pointer format specifier: %%pC\n");
+		return buf;
+	}
+
+	if (!clk)
 		return string(buf, end, NULL, spec);
 
 	switch (fmt[1]) {
@@ -1529,7 +1534,8 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 #ifdef CONFIG_COMMON_CLK
 		return string(buf, end, __clk_get_name(clk), spec);
 #else
-		return special_hex_number(buf, end, (unsigned long)clk, sizeof(unsigned long));
+		WARN_ONCE(1, "Unsupported pointer format specifier: %%pC\n");
+		return buf;
 #endif
 	}
 }
@@ -1593,7 +1599,8 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)
 		names = gfpflag_names;
 		break;
 	default:
-		WARN_ONCE(1, "Unsupported flags modifier: %c\n", fmt[1]);
+		WARN_ONCE(1, "Unsupported pointer format specifier: %%pG%c\n",
+			  fmt[1]);
 		return buf;
 	}
 
-- 
2.13.6

  parent reply	other threads:[~2018-04-04  8:59 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-04  8:58 [PATCH v4 0/9] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
2018-04-04  8:58 ` [PATCH v4 1/9] vsprintf: Shuffle ptr_to_id() code Petr Mladek
2018-04-04  8:58 ` [PATCH v4 2/9] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
2018-04-04 23:10   ` Sergey Senozhatsky
2018-04-05 14:34     ` Petr Mladek
2018-04-05 13:04   ` Andy Shevchenko
2018-04-05 14:46     ` Petr Mladek
2018-04-07 14:08       ` Andy Shevchenko
2018-04-09 12:05         ` Petr Mladek
2018-04-09 12:11           ` Andy Shevchenko
2018-04-04  8:58 ` [PATCH v4 3/9] vsprintf: Do not check address of well-known strings Petr Mladek
2018-04-05 13:30   ` Rasmus Villemoes
2018-04-06  9:15     ` Petr Mladek
2018-04-07 14:12       ` Andy Shevchenko
2018-04-09 12:19         ` Petr Mladek
2018-04-10 10:05           ` Andy Shevchenko
2018-04-04  8:58 ` Petr Mladek [this message]
2018-04-05 14:25   ` [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers Rasmus Villemoes
2018-04-05 23:45     ` Joe Perches
2018-04-05 23:55       ` Joe Perches
2018-04-06 11:43         ` Petr Mladek
2018-04-06 13:17           ` Rasmus Villemoes
2018-04-06 14:27             ` Joe Perches
2018-04-09 12:30               ` Petr Mladek
2018-04-07 14:23             ` Andy Shevchenko
2018-04-06 23:52         ` Sergey Senozhatsky
2018-04-06 23:59           ` Joe Perches
2018-04-07  0:33             ` Sergey Senozhatsky
2018-04-07  1:00               ` Joe Perches
2018-04-07  1:17                 ` Sergey Senozhatsky
2018-04-06 11:25     ` Petr Mladek
2018-04-07 14:26   ` Andy Shevchenko
2018-04-09 13:50     ` Petr Mladek
2018-04-10 11:41       ` Andy Shevchenko
2018-04-11  9:52         ` Petr Mladek
2018-04-24 16:47           ` Andy Shevchenko
2018-04-04  8:58 ` [PATCH v4 5/9] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
2018-04-04 23:58   ` Sergey Senozhatsky
2018-04-05 14:14     ` Petr Mladek
2018-04-07 14:30   ` Andy Shevchenko
2018-04-04  8:58 ` [PATCH v4 6/9] vsprintf: Factor out %pV handler as va_format() Petr Mladek
2018-04-04 14:26   ` Joe Perches
2018-04-06 13:12     ` Petr Mladek
2018-04-06 14:19       ` Joe Perches
2018-04-09 11:44         ` Petr Mladek
2018-04-09 11:59           ` Joe Perches
2018-04-04  8:58 ` [PATCH v4 7/9] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
2018-04-04 23:35   ` Sergey Senozhatsky
2018-04-04 23:43     ` Sergey Senozhatsky
2018-04-05 14:02     ` Petr Mladek
2018-04-04  8:58 ` [PATCH v4 8/9] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
2018-04-05 14:46   ` Rasmus Villemoes
2018-04-06 12:26     ` Petr Mladek
2018-04-06 13:12       ` Rasmus Villemoes
2018-04-10 13:26         ` Petr Mladek
2018-04-06  9:37   ` Rasmus Villemoes
2018-04-04  8:58 ` [PATCH v4 9/9] vsprintf: Avoid confusion between invalid address and value 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=20180404085843.16050-5-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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.