All of 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 05/11] vsprintf: Consolidate handling of unknown pointer specifiers
Date: Wed, 25 Apr 2018 13:12:45 +0200	[thread overview]
Message-ID: <20180425111251.13246-6-pmladek@suse.com> (raw)
In-Reply-To: <20180425111251.13246-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)".

Using WARN() looks like an overkill for this type of error. pr_warn()
is not good either. It would by handled via printk_sage buffer and
it might be hard to match it with the problematic string.

A reasonable compromise seems to be writing the unknown format specifier
into the original string with a question mark, for example (%pC?).
It should be self-explaining enough. Note that it is in brackets
to follow the (null) style.

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

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 5c26a4e71cdf..587175a528b7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1488,7 +1488,8 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 }
 
 static noinline_for_stack
-char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt)
+char *netdev_bits(char *buf, char *end, const void *addr,
+		  struct printf_spec spec, const char *fmt)
 {
 	unsigned long long num;
 	int size;
@@ -1499,9 +1500,7 @@ 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;
+		return valid_string(buf, end, "(%pN?)", spec);
 	}
 
 	return special_hex_number(buf, end, num, size);
@@ -1532,7 +1531,10 @@ 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))
+		return valid_string(buf, end, "(%pC?)", spec);
+
+	if (!clk)
 		return string(buf, end, NULL, spec);
 
 	switch (fmt[1]) {
@@ -1544,7 +1546,7 @@ 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));
+		return valid_string(buf, end, "(%pC?)", spec);
 #endif
 	}
 }
@@ -1577,7 +1579,8 @@ char *format_flags(char *buf, char *end, unsigned long flags,
 }
 
 static noinline_for_stack
-char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)
+char *flags_string(char *buf, char *end, void *flags_ptr,
+		   struct printf_spec spec, const char *fmt)
 {
 	unsigned long flags;
 	const struct trace_print_flags *names;
@@ -1598,8 +1601,7 @@ 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]);
-		return buf;
+		return valid_string(buf, end, "(%pG?)", spec);
 	}
 
 	return format_flags(buf, end, flags, names);
@@ -1655,7 +1657,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	str_spec.field_width = -1;
 
 	if (!IS_ENABLED(CONFIG_OF))
-		return valid_string(buf, end, "(!OF)", spec);
+		return valid_string(buf, end, "(%OF?)", spec);
 
 	if ((unsigned long)dn < PAGE_SIZE)
 		return valid_string(buf, end, "(null)", spec);
@@ -1926,7 +1928,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'K':
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
-		return netdev_bits(buf, end, ptr, fmt);
+		return netdev_bits(buf, end, ptr, spec, fmt);
 	case 'a':
 		return address_val(buf, end, ptr, fmt);
 	case 'd':
@@ -1943,7 +1945,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 #endif
 
 	case 'G':
-		return flags_string(buf, end, ptr, fmt);
+		return flags_string(buf, end, ptr, spec, fmt);
 	case 'O':
 		switch (fmt[1]) {
 		case 'F':
-- 
2.13.6

  parent reply	other threads:[~2018-04-25 11:13 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 ` Petr Mladek [this message]
2018-04-25 13:08   ` [PATCH v5 05/11] vsprintf: Consolidate handling of unknown pointer specifiers 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 ` [PATCH v5 10/11] vsprintf: WARN() on invalid pointer access Petr Mladek
2018-04-25 12:43   ` 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-6-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.