linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling
@ 2019-04-17 11:53 Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 01/10] vsprintf: Shuffle restricted_pointer() Petr Mladek
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

Crash in vsprintf() might be silent when it happens under logbuf_lock
in vprintk_emit(). This patch set prevents most of the crashes by probing
the address. The check is done only by %s and some %p* specifiers that need
to dereference the address.

Only the first byte of the address is checked to keep it simple. It should
be enough to catch most problems.

The check is explicitly done in each function that does the dereference.
It helps to avoid the questionable strchr() of affected specifiers. This
change motivated me to do some preparation patches that consolidated
the error handling and cleaned the code a bit.


Changes against v6:

	+ Removed refactoring of the test_printf code [Andy]
	+ Added missing check_pointer() in device_node_string()
	+ Hard limit on the length of the inlined error msg [Sergey]

Changes against v5:

	+ Rebased on top of current Linus' tree
	+ Removed already included patch adding const qualifier
	  to ptr_to_id()
	+ Removed controversial patch adding WARN() on invalid pointers
	  [Rasmus, Sergey, Andy]
	+ Reshuffled changes to do all refactoring first.
	+ Added handling also for the new %pt* modifiers
	+ Use better descriptive function names [Andy]:
		+ valid_string() -> string_nocheck()
		+ valid_pointer_access() -> check_pointer()
		+ check_pointer_access() -> check_pointer_msg()
	+ Fixed typo and formatting [Andy]

Changes against v4:

	+ rebased on top of
git://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git for-4.18
	+ Added missing conts into ptr_to_ind() in a separate patch
	+ Renamed __string to valid_string()
	+ Avoid WARN() for invalid poimter specifiers
	+ Removed noinline_for_stack where it was not really useful
	+ WARN() when accessing invalid non-NULL address

Changes against v3:

	+ Add valid_pointer_access() to do the check and store the error
	  message in one call.
	+ Remove strchr(). Instead, validate the address in functions
	  that dereference the address.
	+ Use probe_kernel_address() instead of probe_kernel_real().
	+ Do the check only for unknown address.
	+ Consolidate handling of unsupported pointer modifiers.

Changes against v2:

	+ Fix handling with strchr(string, '\0'). Happens with
	  %p at the very end of the string.
	+ Even more clear commit message
	+ Documentation/core-api/printk-formats.rst update.
	+ Add check into lib/test_printf.c.

Changes against v1:

	+ Do not check access for plain %p.
	+ More clear commit message.


Petr Mladek (10):
  vsprintf: Shuffle restricted_pointer()
  vsprintf: Consistent %pK handling for kptr_restrict == 0
  vsprintf: Do not check address of well-known strings
  vsprintf: Factor out %p[iI] handler as ip_addr_string()
  vsprintf: Factor out %pV handler as va_format()
  vsprintf: Factor out %pO handler as kobject_string()
  vsprintf: Consolidate handling of unknown pointer specifiers
  vsprintf: Prevent crash when dereferencing invalid pointers
  vsprintf: Avoid confusion between invalid address and value
  vsprintf: Limit the length of inlined error messages

 Documentation/core-api/printk-formats.rst |   8 +
 lib/test_printf.c                         |  25 +-
 lib/vsprintf.c                            | 426 +++++++++++++++++++-----------
 3 files changed, 298 insertions(+), 161 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v7 01/10] vsprintf: Shuffle restricted_pointer()
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

This is just a preparation step for further changes.

The patch does not change the code.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 98 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 791b6fa36905..eb7b4a06e1f0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -717,6 +717,55 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
 	return pointer_string(buf, end, (const void *)hashval, spec);
 }
 
+int kptr_restrict __read_mostly;
+
+static noinline_for_stack
+char *restricted_pointer(char *buf, char *end, const void *ptr,
+			 struct printf_spec spec)
+{
+	switch (kptr_restrict) {
+	case 0:
+		/* Always print %pK values */
+		break;
+	case 1: {
+		const struct cred *cred;
+
+		/*
+		 * kptr_restrict==1 cannot be used in IRQ context
+		 * because its test for CAP_SYSLOG would be meaningless.
+		 */
+		if (in_irq() || in_serving_softirq() || in_nmi()) {
+			if (spec.field_width == -1)
+				spec.field_width = 2 * sizeof(ptr);
+			return string(buf, end, "pK-error", spec);
+		}
+
+		/*
+		 * Only print the real pointer value if the current
+		 * process has CAP_SYSLOG and is running with the
+		 * same credentials it started with. This is because
+		 * access to files is checked at open() time, but %pK
+		 * checks permission at read() time. We don't want to
+		 * leak pointer values if a binary opens a file using
+		 * %pK and then elevates privileges before reading it.
+		 */
+		cred = current_cred();
+		if (!has_capability_noaudit(current, CAP_SYSLOG) ||
+		    !uid_eq(cred->euid, cred->uid) ||
+		    !gid_eq(cred->egid, cred->gid))
+			ptr = NULL;
+		break;
+	}
+	case 2:
+	default:
+		/* Always print 0's for %pK */
+		ptr = NULL;
+		break;
+	}
+
+	return pointer_string(buf, end, ptr, spec);
+}
+
 static noinline_for_stack
 char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
 		  const char *fmt)
@@ -1476,55 +1525,6 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	return string(buf, end, uuid, spec);
 }
 
-int kptr_restrict __read_mostly;
-
-static noinline_for_stack
-char *restricted_pointer(char *buf, char *end, const void *ptr,
-			 struct printf_spec spec)
-{
-	switch (kptr_restrict) {
-	case 0:
-		/* Always print %pK values */
-		break;
-	case 1: {
-		const struct cred *cred;
-
-		/*
-		 * kptr_restrict==1 cannot be used in IRQ context
-		 * because its test for CAP_SYSLOG would be meaningless.
-		 */
-		if (in_irq() || in_serving_softirq() || in_nmi()) {
-			if (spec.field_width == -1)
-				spec.field_width = 2 * sizeof(ptr);
-			return string(buf, end, "pK-error", spec);
-		}
-
-		/*
-		 * Only print the real pointer value if the current
-		 * process has CAP_SYSLOG and is running with the
-		 * same credentials it started with. This is because
-		 * access to files is checked at open() time, but %pK
-		 * checks permission at read() time. We don't want to
-		 * leak pointer values if a binary opens a file using
-		 * %pK and then elevates privileges before reading it.
-		 */
-		cred = current_cred();
-		if (!has_capability_noaudit(current, CAP_SYSLOG) ||
-		    !uid_eq(cred->euid, cred->uid) ||
-		    !gid_eq(cred->egid, cred->gid))
-			ptr = NULL;
-		break;
-	}
-	case 2:
-	default:
-		/* Always print 0's for %pK */
-		ptr = NULL;
-		break;
-	}
-
-	return pointer_string(buf, end, ptr, spec);
-}
-
 static noinline_for_stack
 char *netdev_bits(char *buf, char *end, const void *addr,
 		  struct printf_spec spec,  const char *fmt)
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 01/10] vsprintf: Shuffle restricted_pointer() Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 13:44   ` Steven Rostedt
  2019-04-17 11:53 ` [PATCH v7 03/10] vsprintf: Do not check address of well-known strings Petr Mladek
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek, Kees Cook

restricted_pointer() pretends that it prints the address when kptr_restrict
is set to zero. But it is never called in this situation. Instead,
pointer() falls back to ptr_to_id() and hashes the pointer.

This patch removes the potential confusion. klp_restrict is checked only
in restricted_pointer().

It actually fixes a small race when the address might get printed unhashed:

CPU0                            CPU1

pointer()
  if (!kptr_restrict)
     /* for example set to 2 */
  restricted_pointer()
				/* echo 0 >/proc/sys/kernel/kptr_restrict */
				proc_dointvec_minmax_sysadmin()
				  klpr_restrict = 0;
    switch(kptr_restrict)
      case 0:
	break:

    number()

Fixes: commit ef0010a30935de4e0211 ("vsprintf: don't use 'restricted_pointer()' when not restricting")
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tobin Harding <me@tobin.cc>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index eb7b4a06e1f0..2af48948a973 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -725,8 +725,8 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
 {
 	switch (kptr_restrict) {
 	case 0:
-		/* Always print %pK values */
-		break;
+		/* Handle as %p, hash and do _not_ leak addresses. */
+		return ptr_to_id(buf, end, ptr, spec);
 	case 1: {
 		const struct cred *cred;
 
@@ -2041,8 +2041,6 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 			return buf;
 		}
 	case 'K':
-		if (!kptr_restrict)
-			break;
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
 		return netdev_bits(buf, end, ptr, spec, fmt);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 03/10] vsprintf: Do not check address of well-known strings
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 01/10] vsprintf: Shuffle restricted_pointer() Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 04/10] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

We are going to check the address using probe_kernel_address(). It will
be more expensive and it does not make sense for well known address.

This patch splits the string() function. The variant without the check
is then used on locations that handle string constants or strings defined
as local variables.

This patch does not change the existing behavior.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 81 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 44 insertions(+), 37 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2af48948a973..c9c9a1179870 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -593,15 +593,13 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 	return buf;
 }
 
-static noinline_for_stack
-char *string(char *buf, char *end, const char *s, struct printf_spec spec)
+/* Handle string from a well known address. */
+static char *string_nocheck(char *buf, char *end, const char *s,
+			    struct printf_spec spec)
 {
 	int len = 0;
 	size_t lim = spec.precision;
 
-	if ((unsigned long)s < PAGE_SIZE)
-		s = "(null)";
-
 	while (lim--) {
 		char c = *s++;
 		if (!c)
@@ -615,6 +613,15 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 }
 
 static noinline_for_stack
+char *string(char *buf, char *end, const char *s,
+	     struct printf_spec spec)
+{
+	if ((unsigned long)s < PAGE_SIZE)
+		s = "(null)";
+
+	return string_nocheck(buf, end, s, spec);
+}
+
 char *pointer_string(char *buf, char *end, const void *ptr,
 		     struct printf_spec spec)
 {
@@ -701,7 +708,7 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
 	if (static_branch_unlikely(&not_filled_random_ptr_key)) {
 		spec.field_width = 2 * sizeof(ptr);
 		/* string length must be less than default_width */
-		return string(buf, end, str, spec);
+		return string_nocheck(buf, end, str, spec);
 	}
 
 #ifdef CONFIG_64BIT
@@ -737,7 +744,7 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
 		if (in_irq() || in_serving_softirq() || in_nmi()) {
 			if (spec.field_width == -1)
 				spec.field_width = 2 * sizeof(ptr);
-			return string(buf, end, "pK-error", spec);
+			return string_nocheck(buf, end, "pK-error", spec);
 		}
 
 		/*
@@ -851,7 +858,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 	else
 		sprint_symbol_no_offset(sym, value);
 
-	return string(buf, end, sym, spec);
+	return string_nocheck(buf, end, sym, spec);
 #else
 	return special_hex_number(buf, end, value, sizeof(void *));
 #endif
@@ -937,27 +944,27 @@ char *resource_string(char *buf, char *end, struct resource *res,
 
 	*p++ = '[';
 	if (res->flags & IORESOURCE_IO) {
-		p = string(p, pend, "io  ", str_spec);
+		p = string_nocheck(p, pend, "io  ", str_spec);
 		specp = &io_spec;
 	} else if (res->flags & IORESOURCE_MEM) {
-		p = string(p, pend, "mem ", str_spec);
+		p = string_nocheck(p, pend, "mem ", str_spec);
 		specp = &mem_spec;
 	} else if (res->flags & IORESOURCE_IRQ) {
-		p = string(p, pend, "irq ", str_spec);
+		p = string_nocheck(p, pend, "irq ", str_spec);
 		specp = &default_dec_spec;
 	} else if (res->flags & IORESOURCE_DMA) {
-		p = string(p, pend, "dma ", str_spec);
+		p = string_nocheck(p, pend, "dma ", str_spec);
 		specp = &default_dec_spec;
 	} else if (res->flags & IORESOURCE_BUS) {
-		p = string(p, pend, "bus ", str_spec);
+		p = string_nocheck(p, pend, "bus ", str_spec);
 		specp = &bus_spec;
 	} else {
-		p = string(p, pend, "??? ", str_spec);
+		p = string_nocheck(p, pend, "??? ", str_spec);
 		specp = &mem_spec;
 		decode = 0;
 	}
 	if (decode && res->flags & IORESOURCE_UNSET) {
-		p = string(p, pend, "size ", str_spec);
+		p = string_nocheck(p, pend, "size ", str_spec);
 		p = number(p, pend, resource_size(res), *specp);
 	} else {
 		p = number(p, pend, res->start, *specp);
@@ -968,21 +975,21 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	}
 	if (decode) {
 		if (res->flags & IORESOURCE_MEM_64)
-			p = string(p, pend, " 64bit", str_spec);
+			p = string_nocheck(p, pend, " 64bit", str_spec);
 		if (res->flags & IORESOURCE_PREFETCH)
-			p = string(p, pend, " pref", str_spec);
+			p = string_nocheck(p, pend, " pref", str_spec);
 		if (res->flags & IORESOURCE_WINDOW)
-			p = string(p, pend, " window", str_spec);
+			p = string_nocheck(p, pend, " window", str_spec);
 		if (res->flags & IORESOURCE_DISABLED)
-			p = string(p, pend, " disabled", str_spec);
+			p = string_nocheck(p, pend, " disabled", str_spec);
 	} else {
-		p = string(p, pend, " flags ", str_spec);
+		p = string_nocheck(p, pend, " flags ", str_spec);
 		p = number(p, pend, res->flags, default_flag_spec);
 	}
 	*p++ = ']';
 	*p = '\0';
 
-	return string(buf, end, sym, spec);
+	return string_nocheck(buf, end, sym, spec);
 }
 
 static noinline_for_stack
@@ -1150,7 +1157,7 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
 	}
 	*p = '\0';
 
-	return string(buf, end, mac_addr, spec);
+	return string_nocheck(buf, end, mac_addr, spec);
 }
 
 static noinline_for_stack
@@ -1313,7 +1320,7 @@ char *ip6_addr_string(char *buf, char *end, const u8 *addr,
 	else
 		ip6_string(ip6_addr, addr, fmt);
 
-	return string(buf, end, ip6_addr, spec);
+	return string_nocheck(buf, end, ip6_addr, spec);
 }
 
 static noinline_for_stack
@@ -1324,7 +1331,7 @@ char *ip4_addr_string(char *buf, char *end, const u8 *addr,
 
 	ip4_string(ip4_addr, addr, fmt);
 
-	return string(buf, end, ip4_addr, spec);
+	return string_nocheck(buf, end, ip4_addr, spec);
 }
 
 static noinline_for_stack
@@ -1386,7 +1393,7 @@ char *ip6_addr_string_sa(char *buf, char *end, const struct sockaddr_in6 *sa,
 	}
 	*p = '\0';
 
-	return string(buf, end, ip6_addr, spec);
+	return string_nocheck(buf, end, ip6_addr, spec);
 }
 
 static noinline_for_stack
@@ -1421,7 +1428,7 @@ char *ip4_addr_string_sa(char *buf, char *end, const struct sockaddr_in *sa,
 	}
 	*p = '\0';
 
-	return string(buf, end, ip4_addr, spec);
+	return string_nocheck(buf, end, ip4_addr, spec);
 }
 
 static noinline_for_stack
@@ -1522,7 +1529,7 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 
 	*p = 0;
 
-	return string(buf, end, uuid, spec);
+	return string_nocheck(buf, end, uuid, spec);
 }
 
 static noinline_for_stack
@@ -1736,13 +1743,13 @@ char *device_node_gen_full_name(const struct device_node *np, char *buf, char *e
 
 	/* special case for root node */
 	if (!parent)
-		return string(buf, end, "/", default_str_spec);
+		return string_nocheck(buf, end, "/", default_str_spec);
 
 	for (depth = 0; parent->parent; depth++)
 		parent = parent->parent;
 
 	for ( ; depth >= 0; depth--) {
-		buf = string(buf, end, "/", default_str_spec);
+		buf = string_nocheck(buf, end, "/", default_str_spec);
 		buf = string(buf, end, device_node_name_for_depth(np, depth),
 			     default_str_spec);
 	}
@@ -1770,10 +1777,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	str_spec.field_width = -1;
 
 	if (!IS_ENABLED(CONFIG_OF))
-		return string(buf, end, "(!OF)", spec);
+		return string_nocheck(buf, end, "(!OF)", spec);
 
 	if ((unsigned long)dn < PAGE_SIZE)
-		return string(buf, end, "(null)", spec);
+		return string_nocheck(buf, end, "(null)", spec);
 
 	/* simple case without anything any more format specifiers */
 	fmt++;
@@ -1814,7 +1821,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 			tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-';
 			tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-';
 			tbuf[4] = 0;
-			buf = string(buf, end, tbuf, str_spec);
+			buf = string_nocheck(buf, end, tbuf, str_spec);
 			break;
 		case 'c':	/* major compatible string */
 			ret = of_property_read_string(dn, "compatible", &p);
@@ -1825,10 +1832,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 			has_mult = false;
 			of_property_for_each_string(dn, "compatible", prop, p) {
 				if (has_mult)
-					buf = string(buf, end, ",", str_spec);
-				buf = string(buf, end, "\"", str_spec);
+					buf = string_nocheck(buf, end, ",", str_spec);
+				buf = string_nocheck(buf, end, "\"", str_spec);
 				buf = string(buf, end, p, str_spec);
-				buf = string(buf, end, "\"", str_spec);
+				buf = string_nocheck(buf, end, "\"", str_spec);
 
 				has_mult = true;
 			}
@@ -1966,7 +1973,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		 */
 		if (spec.field_width == -1)
 			spec.field_width = default_width;
-		return string(buf, end, "(null)", spec);
+		return string_nocheck(buf, end, "(null)", spec);
 	}
 
 	switch (*fmt) {
@@ -2022,7 +2029,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 			case AF_INET6:
 				return ip6_addr_string_sa(buf, end, &sa->v6, spec, fmt);
 			default:
-				return string(buf, end, "(invalid address)", spec);
+				return string_nocheck(buf, end, "(invalid address)", spec);
 			}}
 		}
 		break;
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 04/10] vsprintf: Factor out %p[iI] handler as ip_addr_string()
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (2 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 03/10] vsprintf: Do not check address of well-known strings Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 05/10] vsprintf: Factor out %pV handler as va_format() Petr Mladek
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

Move the non-trivial code from the long pointer() function. We are going
to improve error handling that will make it even more complicated.

This patch does not change the existing behavior.

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

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index c9c9a1179870..8ca29bc0d786 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1431,6 +1431,35 @@ char *ip4_addr_string_sa(char *buf, char *end, const struct sockaddr_in *sa,
 	return string_nocheck(buf, end, ip4_addr, spec);
 }
 
+static noinline_for_stack
+char *ip_addr_string(char *buf, char *end, const void *ptr,
+		     struct printf_spec spec, const char *fmt)
+{
+	switch (fmt[1]) {
+	case '6':
+		return ip6_addr_string(buf, end, ptr, spec, fmt);
+	case '4':
+		return ip4_addr_string(buf, end, ptr, spec, fmt);
+	case 'S': {
+		const union {
+			struct sockaddr		raw;
+			struct sockaddr_in	v4;
+			struct sockaddr_in6	v6;
+		} *sa = ptr;
+
+		switch (sa->raw.sa_family) {
+		case AF_INET:
+			return ip4_addr_string_sa(buf, end, &sa->v4, spec, fmt);
+		case AF_INET6:
+			return ip6_addr_string_sa(buf, end, &sa->v6, spec, fmt);
+		default:
+			return string_nocheck(buf, end, "(invalid address)", spec);
+		}}
+	}
+
+	return ptr_to_id(buf, end, ptr, spec);
+}
+
 static noinline_for_stack
 char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 		     const char *fmt)
@@ -2011,28 +2040,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 					 * 4:	001.002.003.004
 					 * 6:   000102...0f
 					 */
-		switch (fmt[1]) {
-		case '6':
-			return ip6_addr_string(buf, end, ptr, spec, fmt);
-		case '4':
-			return ip4_addr_string(buf, end, ptr, spec, fmt);
-		case 'S': {
-			const union {
-				struct sockaddr		raw;
-				struct sockaddr_in	v4;
-				struct sockaddr_in6	v6;
-			} *sa = ptr;
-
-			switch (sa->raw.sa_family) {
-			case AF_INET:
-				return ip4_addr_string_sa(buf, end, &sa->v4, spec, fmt);
-			case AF_INET6:
-				return ip6_addr_string_sa(buf, end, &sa->v6, spec, fmt);
-			default:
-				return string_nocheck(buf, end, "(invalid address)", spec);
-			}}
-		}
-		break;
+		return ip_addr_string(buf, end, ptr, spec, fmt);
 	case 'E':
 		return escaped_string(buf, end, ptr, spec, fmt);
 	case 'U':
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 05/10] vsprintf: Factor out %pV handler as va_format()
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (3 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 04/10] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 06/10] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

Move the code from the long pointer() function. We are going to improve
error handling that will make it more complicated.

This patch does not change the existing behavior.

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

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 8ca29bc0d786..12b71a4d4613 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1520,6 +1520,17 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 	return buf;
 }
 
+static char *va_format(char *buf, char *end, struct va_format *va_fmt)
+{
+	va_list va;
+
+	va_copy(va, *va_fmt->va);
+	buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
+	va_end(va);
+
+	return buf;
+}
+
 static noinline_for_stack
 char *uuid_string(char *buf, char *end, const u8 *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2046,15 +2057,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'U':
 		return uuid_string(buf, end, ptr, spec, fmt);
 	case 'V':
-		{
-			va_list va;
-
-			va_copy(va, *((struct va_format *)ptr)->va);
-			buf += vsnprintf(buf, end > buf ? end - buf : 0,
-					 ((struct va_format *)ptr)->fmt, va);
-			va_end(va);
-			return buf;
-		}
+		return va_format(buf, end, ptr);
 	case 'K':
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 06/10] vsprintf: Factor out %pO handler as kobject_string()
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (4 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 05/10] vsprintf: Factor out %pV handler as va_format() Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek, Kees Cook

Move code from the long pointer() function. We are going to improve
error handling that will make it even more complicated.

This patch does not change the existing behavior.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tobin Harding <me@tobin.cc>
Cc: Kees Cook <keescook@chromium.org>
---
 lib/vsprintf.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 12b71a4d4613..9817d171f608 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1888,6 +1888,17 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	return widen_string(buf, buf - buf_start, end, spec);
 }
 
+static char *kobject_string(char *buf, char *end, void *ptr,
+			    struct printf_spec spec, const char *fmt)
+{
+	switch (fmt[1]) {
+	case 'F':
+		return device_node_string(buf, end, ptr, spec, fmt + 1);
+	}
+
+	return ptr_to_id(buf, end, ptr, spec);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -2082,11 +2093,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'G':
 		return flags_string(buf, end, ptr, fmt);
 	case 'O':
-		switch (fmt[1]) {
-		case 'F':
-			return device_node_string(buf, end, ptr, spec, fmt + 1);
-		}
-		break;
+		return kobject_string(buf, end, ptr, spec, fmt);
 	case 'x':
 		return pointer_string(buf, end, ptr, spec);
 	}
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (5 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 06/10] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-18 14:34   ` Sergey Senozhatsky
                     ` (3 more replies)
  2019-04-17 11:53 ` [PATCH v7 08/10] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
                   ` (3 subsequent siblings)
  10 siblings, 4 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

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 inconsistent and not helpful.
Using WARN() looks like an overkill for this type of error. pr_warn()
is not good either. It would by handled via printk_safe 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.

Note that it introduces a warning about that test_hashed() function
is unused. It is going to be used again by a later patch.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/test_printf.c |  3 +--
 lib/vsprintf.c    | 28 +++++++++++++++++-----------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 659b6cc0d483..250ee864b8b8 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -462,8 +462,7 @@ struct_rtc_time(void)
 		.tm_year = 118,
 	};
 
-	test_hashed("%pt", &tm);
-
+	test("(%ptR?)", "%pt", &tm);
 	test("2018-11-26T05:35:43", "%ptR", &tm);
 	test("0118-10-26T05:35:43", "%ptRr", &tm);
 	test("05:35:43|2018-11-26", "%ptRt|%ptRd", &tm, &tm);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9817d171f608..f471a658422f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1435,6 +1435,8 @@ static noinline_for_stack
 char *ip_addr_string(char *buf, char *end, const void *ptr,
 		     struct printf_spec spec, const char *fmt)
 {
+	char *err_fmt_msg;
+
 	switch (fmt[1]) {
 	case '6':
 		return ip6_addr_string(buf, end, ptr, spec, fmt);
@@ -1457,7 +1459,8 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
 		}}
 	}
 
-	return ptr_to_id(buf, end, ptr, spec);
+	err_fmt_msg = fmt[0] == 'i' ? "(%pi?)" : "(%pI?)";
+	return string_nocheck(buf, end, err_fmt_msg, spec);
 }
 
 static noinline_for_stack
@@ -1585,7 +1588,7 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 		size = sizeof(netdev_features_t);
 		break;
 	default:
-		return ptr_to_id(buf, end, addr, spec);
+		return string_nocheck(buf, end, "(%pN?)", spec);
 	}
 
 	return special_hex_number(buf, end, num, size);
@@ -1689,7 +1692,7 @@ char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
 	case 'R':
 		return rtc_str(buf, end, (const struct rtc_time *)ptr, fmt);
 	default:
-		return ptr_to_id(buf, end, ptr, spec);
+		return string_nocheck(buf, end, "(%ptR?)", spec);
 	}
 }
 
@@ -1697,7 +1700,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 string_nocheck(buf, end, "(%pC?)", spec);
+
+	if (!clk)
 		return string(buf, end, NULL, spec);
 
 	switch (fmt[1]) {
@@ -1706,7 +1712,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 ptr_to_id(buf, end, clk, spec);
+		return string_nocheck(buf, end, "(%pC?)", spec);
 #endif
 	}
 }
@@ -1739,7 +1745,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;
@@ -1760,8 +1767,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 string_nocheck(buf, end, "(%pG?)", spec);
 	}
 
 	return format_flags(buf, end, flags, names);
@@ -1817,7 +1823,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	str_spec.field_width = -1;
 
 	if (!IS_ENABLED(CONFIG_OF))
-		return string_nocheck(buf, end, "(!OF)", spec);
+		return string_nocheck(buf, end, "(%pOF?)", spec);
 
 	if ((unsigned long)dn < PAGE_SIZE)
 		return string_nocheck(buf, end, "(null)", spec);
@@ -1896,7 +1902,7 @@ static char *kobject_string(char *buf, char *end, void *ptr,
 		return device_node_string(buf, end, ptr, spec, fmt + 1);
 	}
 
-	return ptr_to_id(buf, end, ptr, spec);
+	return string_nocheck(buf, end, "(%pO?)", spec);
 }
 
 /*
@@ -2091,7 +2097,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':
 		return kobject_string(buf, end, ptr, spec, fmt);
 	case 'x':
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 08/10] vsprintf: Prevent crash when dereferencing invalid pointers
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (6 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 09/10] vsprintf: Avoid confusion between invalid address and value Petr Mladek
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

We already prevent crash when dereferencing some obviously broken
pointers. But the handling is not consistent. Sometimes we print "(null)"
only for pure NULL pointer, sometimes for pointers in the first
page and sometimes also for pointers in the last page (error codes).

Note that printk() call this code under logbuf_lock. Any recursive
printks are redirected to the printk_safe implementation and the messages
are stored into per-CPU buffers. These buffers might be eventually flushed
in printk_safe_flush_on_panic() but it is not guaranteed.

This patch adds a check using probe_kernel_read(). It is not a full-proof
test. But it should help to see the error message in 99% situations where
the kernel would silently crash otherwise.

Also it makes the error handling unified for "%s" and the many %p*
specifiers that need to read the data from a given address. We print:

   + (null)   when accessing data on pure pure NULL address
   + (efault) when accessing data on an invalid address

It does not affect the %p* specifiers that just print the given address
in some form, namely %pF, %pf, %pS, %ps, %pB, %pK, %px, and plain %p.

Note that we print (efault) from security reasons. In fact, the real
address can be seen only by %px or eventually %pK.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 Documentation/core-api/printk-formats.rst |   7 ++
 lib/test_printf.c                         |  22 ++++-
 lib/vsprintf.c                            | 136 ++++++++++++++++++++++--------
 3 files changed, 129 insertions(+), 36 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index c37ec7cd9c06..b2cac8d76b66 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -58,6 +58,13 @@ A raw pointer value may be printed with %p which will hash the address
 before printing. The kernel also supports extended specifiers for printing
 pointers of different types.
 
+Some of the extended specifiers print the data on the given address instead
+of printing the address itself. In this case, the following error messages
+might be printed instead of the unreachable information::
+
+	(null)	 data on plain NULL address
+	(efault) data on invalid address
+
 Plain Pointers
 --------------
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 250ee864b8b8..359ae4fb1ece 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -239,6 +239,7 @@ plain_format(void)
 #define PTR ((void *)0x456789ab)
 #define PTR_STR "456789ab"
 #define PTR_VAL_NO_CRNG "(ptrval)"
+#define ZEROS ""
 
 static int __init
 plain_format(void)
@@ -268,7 +269,6 @@ plain_hash_to_buffer(const void *p, char *buf, size_t len)
 	return 0;
 }
 
-
 static int __init
 plain_hash(void)
 {
@@ -325,6 +325,24 @@ test_hashed(const char *fmt, const void *p)
 	test(buf, fmt, p);
 }
 
+static void __init
+null_pointer(void)
+{
+	test_hashed("%p", NULL);
+	test(ZEROS "00000000", "%px", NULL);
+	test("(null)", "%pE", NULL);
+}
+
+#define PTR_INVALID ((void *)0x000000ab)
+
+static void __init
+invalid_pointer(void)
+{
+	test_hashed("%p", PTR_INVALID);
+	test(ZEROS "000000ab", "%px", PTR_INVALID);
+	test("(efault)", "%pE", PTR_INVALID);
+}
+
 static void __init
 symbol_ptr(void)
 {
@@ -571,6 +589,8 @@ static void __init
 test_pointer(void)
 {
 	plain();
+	null_pointer();
+	invalid_pointer();
 	symbol_ptr();
 	kernel_ptr();
 	struct_resource();
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f471a658422f..b989f1e8f35b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -612,12 +612,45 @@ static char *string_nocheck(char *buf, char *end, const char *s,
 	return widen_string(buf, len, end, spec);
 }
 
+/*
+ * 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
+ * the address to make sure it doesn't fault due to a poorly added printk
+ * during debugging.
+ */
+static const char *check_pointer_msg(const void *ptr)
+{
+	char byte;
+
+	if (!ptr)
+		return "(null)";
+
+	if (probe_kernel_address(ptr, byte))
+		return "(efault)";
+
+	return NULL;
+}
+
+static int check_pointer(char **buf, char *end, const void *ptr,
+			 struct printf_spec spec)
+{
+	const char *err_msg;
+
+	err_msg = check_pointer_msg(ptr);
+	if (err_msg) {
+		*buf = string_nocheck(*buf, end, err_msg, spec);
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
 static noinline_for_stack
 char *string(char *buf, char *end, const char *s,
 	     struct printf_spec spec)
 {
-	if ((unsigned long)s < PAGE_SIZE)
-		s = "(null)";
+	if (check_pointer(&buf, end, s, spec))
+		return buf;
 
 	return string_nocheck(buf, end, s, spec);
 }
@@ -792,6 +825,11 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
 
 	rcu_read_lock();
 	for (i = 0; i < depth; i++, d = p) {
+		if (check_pointer(&buf, end, d, spec)) {
+			rcu_read_unlock();
+			return buf;
+		}
+
 		p = READ_ONCE(d->d_parent);
 		array[i] = READ_ONCE(d->d_name.name);
 		if (p == d) {
@@ -822,8 +860,12 @@ static noinline_for_stack
 char *bdev_name(char *buf, char *end, struct block_device *bdev,
 		struct printf_spec spec, const char *fmt)
 {
-	struct gendisk *hd = bdev->bd_disk;
-	
+	struct gendisk *hd;
+
+	if (check_pointer(&buf, end, bdev, spec))
+		return buf;
+
+	hd = bdev->bd_disk;
 	buf = string(buf, end, hd->disk_name, spec);
 	if (bdev->bd_part->partno) {
 		if (isdigit(hd->disk_name[strlen(hd->disk_name)-1])) {
@@ -942,6 +984,9 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	int decode = (fmt[0] == 'R') ? 1 : 0;
 	const struct printf_spec *specp;
 
+	if (check_pointer(&buf, end, res, spec))
+		return buf;
+
 	*p++ = '[';
 	if (res->flags & IORESOURCE_IO) {
 		p = string_nocheck(p, pend, "io  ", str_spec);
@@ -1004,9 +1049,8 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 		/* nothing to print */
 		return buf;
 
-	if (ZERO_OR_NULL_PTR(addr))
-		/* NULL pointer */
-		return string(buf, end, NULL, spec);
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
 
 	switch (fmt[1]) {
 	case 'C':
@@ -1053,6 +1097,9 @@ char *bitmap_string(char *buf, char *end, unsigned long *bitmap,
 	int i, chunksz;
 	bool first = true;
 
+	if (check_pointer(&buf, end, bitmap, spec))
+		return buf;
+
 	/* reused to print numbers */
 	spec = (struct printf_spec){ .flags = SMALL | ZEROPAD, .base = 16 };
 
@@ -1094,6 +1141,9 @@ char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap,
 	int cur, rbot, rtop;
 	bool first = true;
 
+	if (check_pointer(&buf, end, bitmap, spec))
+		return buf;
+
 	rbot = cur = find_first_bit(bitmap, nr_bits);
 	while (cur < nr_bits) {
 		rtop = cur;
@@ -1132,6 +1182,9 @@ char *mac_address_string(char *buf, char *end, u8 *addr,
 	char separator;
 	bool reversed = false;
 
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
+
 	switch (fmt[1]) {
 	case 'F':
 		separator = '-';
@@ -1437,6 +1490,9 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
 {
 	char *err_fmt_msg;
 
+	if (check_pointer(&buf, end, ptr, spec))
+		return buf;
+
 	switch (fmt[1]) {
 	case '6':
 		return ip6_addr_string(buf, end, ptr, spec, fmt);
@@ -1475,9 +1531,8 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 	if (spec.field_width == 0)
 		return buf;				/* nothing to print */
 
-	if (ZERO_OR_NULL_PTR(addr))
-		return string(buf, end, NULL, spec);	/* NULL pointer */
-
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
 
 	do {
 		switch (fmt[count++]) {
@@ -1523,10 +1578,14 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 	return buf;
 }
 
-static char *va_format(char *buf, char *end, struct va_format *va_fmt)
+static char *va_format(char *buf, char *end, struct va_format *va_fmt,
+		       struct printf_spec spec, const char *fmt)
 {
 	va_list va;
 
+	if (check_pointer(&buf, end, va_fmt, spec))
+		return buf;
+
 	va_copy(va, *va_fmt->va);
 	buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
 	va_end(va);
@@ -1544,6 +1603,9 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	const u8 *index = uuid_index;
 	bool uc = false;
 
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
+
 	switch (*(++fmt)) {
 	case 'L':
 		uc = true;		/* fall-through */
@@ -1582,6 +1644,9 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 	unsigned long long num;
 	int size;
 
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
+
 	switch (fmt[1]) {
 	case 'F':
 		num = *(const netdev_features_t *)addr;
@@ -1595,11 +1660,15 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 }
 
 static noinline_for_stack
-char *address_val(char *buf, char *end, const void *addr, const char *fmt)
+char *address_val(char *buf, char *end, const void *addr,
+		  struct printf_spec spec, const char *fmt)
 {
 	unsigned long long num;
 	int size;
 
+	if (check_pointer(&buf, end, addr, spec))
+		return buf;
+
 	switch (fmt[1]) {
 	case 'd':
 		num = *(const dma_addr_t *)addr;
@@ -1651,12 +1720,16 @@ char *time_str(char *buf, char *end, const struct rtc_time *tm, bool r)
 }
 
 static noinline_for_stack
-char *rtc_str(char *buf, char *end, const struct rtc_time *tm, const char *fmt)
+char *rtc_str(char *buf, char *end, const struct rtc_time *tm,
+	      struct printf_spec spec, const char *fmt)
 {
 	bool have_t = true, have_d = true;
 	bool raw = false;
 	int count = 2;
 
+	if (check_pointer(&buf, end, tm, spec))
+		return buf;
+
 	switch (fmt[count]) {
 	case 'd':
 		have_t = false;
@@ -1690,7 +1763,7 @@ char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
 {
 	switch (fmt[1]) {
 	case 'R':
-		return rtc_str(buf, end, (const struct rtc_time *)ptr, fmt);
+		return rtc_str(buf, end, (const struct rtc_time *)ptr, spec, fmt);
 	default:
 		return string_nocheck(buf, end, "(%ptR?)", spec);
 	}
@@ -1703,8 +1776,8 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 	if (!IS_ENABLED(CONFIG_HAVE_CLK))
 		return string_nocheck(buf, end, "(%pC?)", spec);
 
-	if (!clk)
-		return string(buf, end, NULL, spec);
+	if (check_pointer(&buf, end, clk, spec))
+		return buf;
 
 	switch (fmt[1]) {
 	case 'n':
@@ -1751,6 +1824,9 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
 	unsigned long flags;
 	const struct trace_print_flags *names;
 
+	if (check_pointer(&buf, end, flags_ptr, spec))
+		return buf;
+
 	switch (fmt[1]) {
 	case 'p':
 		flags = *(unsigned long *)flags_ptr;
@@ -1825,8 +1901,8 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	if (!IS_ENABLED(CONFIG_OF))
 		return string_nocheck(buf, end, "(%pOF?)", spec);
 
-	if ((unsigned long)dn < PAGE_SIZE)
-		return string_nocheck(buf, end, "(null)", spec);
+	if (check_pointer(&buf, end, dn, spec))
+		return buf;
 
 	/* simple case without anything any more format specifiers */
 	fmt++;
@@ -2021,18 +2097,6 @@ static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	      struct printf_spec spec)
 {
-	const int default_width = 2 * sizeof(void *);
-
-	if (!ptr && *fmt != 'K' && *fmt != 'x') {
-		/*
-		 * Print (null) with the same width as a pointer so it makes
-		 * tabular output look nice.
-		 */
-		if (spec.field_width == -1)
-			spec.field_width = default_width;
-		return string_nocheck(buf, end, "(null)", spec);
-	}
-
 	switch (*fmt) {
 	case 'F':
 	case 'f':
@@ -2074,13 +2138,13 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'U':
 		return uuid_string(buf, end, ptr, spec, fmt);
 	case 'V':
-		return va_format(buf, end, ptr);
+		return va_format(buf, end, ptr, spec, fmt);
 	case 'K':
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
 		return netdev_bits(buf, end, ptr, spec, fmt);
 	case 'a':
-		return address_val(buf, end, ptr, fmt);
+		return address_val(buf, end, ptr, spec, fmt);
 	case 'd':
 		return dentry_name(buf, end, ptr, spec, fmt);
 	case 't':
@@ -2714,11 +2778,13 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args)
 
 		case FORMAT_TYPE_STR: {
 			const char *save_str = va_arg(args, char *);
+			const char *err_msg;
 			size_t len;
 
-			if ((unsigned long)save_str > (unsigned long)-PAGE_SIZE
-					|| (unsigned long)save_str < PAGE_SIZE)
-				save_str = "(null)";
+			err_msg = check_pointer_msg(save_str);
+			if (err_msg)
+				save_str = err_msg;
+
 			len = strlen(save_str) + 1;
 			if (str + len < end)
 				memcpy(str, save_str, len);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 09/10] vsprintf: Avoid confusion between invalid address and value
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (7 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 08/10] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-17 11:53 ` [PATCH v7 10/10] vsprintf: Limit the length of inlined error messages Petr Mladek
  2019-04-19  1:51 ` [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Sergey Senozhatsky
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

We are able to detect invalid values handled by %p[iI] printk specifier.
The current error message is "invalid address". It might cause confusion
against "(efault)" reported by the generic valid_pointer_address() check.

Let's unify the style and use the more appropriate error code description
"(einval)".

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

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index b2cac8d76b66..75d2bbe9813f 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -64,6 +64,7 @@ of printing the address itself. In this case, the following error messages
 
 	(null)	 data on plain NULL address
 	(efault) data on invalid address
+	(einval) invalid data on a valid address
 
 Plain Pointers
 --------------
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b989f1e8f35b..4e5666035b74 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1511,7 +1511,7 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
 		case AF_INET6:
 			return ip6_addr_string_sa(buf, end, &sa->v6, spec, fmt);
 		default:
-			return string_nocheck(buf, end, "(invalid address)", spec);
+			return string_nocheck(buf, end, "(einval)", spec);
 		}}
 	}
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v7 10/10] vsprintf: Limit the length of inlined error messages
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (8 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 09/10] vsprintf: Avoid confusion between invalid address and value Petr Mladek
@ 2019-04-17 11:53 ` Petr Mladek
  2019-04-19  1:51 ` [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Sergey Senozhatsky
  10 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-17 11:53 UTC (permalink / raw)
  To: Andy Shevchenko, Rasmus Villemoes
  Cc: Linus Torvalds, Tobin C . Harding, Joe Perches, Andrew Morton,
	Michal Hocko, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky, linux-kernel, Petr Mladek

The inlined error messages must be used carefully because
they need to fit into the given buffer.

Handle them using a custom wrapper that makes people aware
of the problem. Also define a reasonable hard limit to
avoid a completely insane usage.

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 lib/vsprintf.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 4e5666035b74..1f367f3a7e2b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -612,6 +612,21 @@ static char *string_nocheck(char *buf, char *end, const char *s,
 	return widen_string(buf, len, end, spec);
 }
 
+/* Be careful: error messages must fit into the given buffer. */
+static char *error_string(char *buf, char *end, const char *s,
+			  struct printf_spec spec)
+{
+	/*
+	 * Hard limit to avoid a completely insane messages. It actually
+	 * works pretty well because most error messages are in
+	 * the many pointer format modifiers.
+	 */
+	if (spec.precision == -1)
+		spec.precision = 2 * sizeof(void *);
+
+	return string_nocheck(buf, end, s, spec);
+}
+
 /*
  * 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
@@ -638,7 +653,7 @@ static int check_pointer(char **buf, char *end, const void *ptr,
 
 	err_msg = check_pointer_msg(ptr);
 	if (err_msg) {
-		*buf = string_nocheck(*buf, end, err_msg, spec);
+		*buf = error_string(*buf, end, err_msg, spec);
 		return -EFAULT;
 	}
 
@@ -741,7 +756,7 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
 	if (static_branch_unlikely(&not_filled_random_ptr_key)) {
 		spec.field_width = 2 * sizeof(ptr);
 		/* string length must be less than default_width */
-		return string_nocheck(buf, end, str, spec);
+		return error_string(buf, end, str, spec);
 	}
 
 #ifdef CONFIG_64BIT
@@ -777,7 +792,7 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
 		if (in_irq() || in_serving_softirq() || in_nmi()) {
 			if (spec.field_width == -1)
 				spec.field_width = 2 * sizeof(ptr);
-			return string_nocheck(buf, end, "pK-error", spec);
+			return error_string(buf, end, "pK-error", spec);
 		}
 
 		/*
@@ -1511,12 +1526,12 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
 		case AF_INET6:
 			return ip6_addr_string_sa(buf, end, &sa->v6, spec, fmt);
 		default:
-			return string_nocheck(buf, end, "(einval)", spec);
+			return error_string(buf, end, "(einval)", spec);
 		}}
 	}
 
 	err_fmt_msg = fmt[0] == 'i' ? "(%pi?)" : "(%pI?)";
-	return string_nocheck(buf, end, err_fmt_msg, spec);
+	return error_string(buf, end, err_fmt_msg, spec);
 }
 
 static noinline_for_stack
@@ -1653,7 +1668,7 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 		size = sizeof(netdev_features_t);
 		break;
 	default:
-		return string_nocheck(buf, end, "(%pN?)", spec);
+		return error_string(buf, end, "(%pN?)", spec);
 	}
 
 	return special_hex_number(buf, end, num, size);
@@ -1765,7 +1780,7 @@ char *time_and_date(char *buf, char *end, void *ptr, struct printf_spec spec,
 	case 'R':
 		return rtc_str(buf, end, (const struct rtc_time *)ptr, spec, fmt);
 	default:
-		return string_nocheck(buf, end, "(%ptR?)", spec);
+		return error_string(buf, end, "(%ptR?)", spec);
 	}
 }
 
@@ -1774,7 +1789,7 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec,
 	    const char *fmt)
 {
 	if (!IS_ENABLED(CONFIG_HAVE_CLK))
-		return string_nocheck(buf, end, "(%pC?)", spec);
+		return error_string(buf, end, "(%pC?)", spec);
 
 	if (check_pointer(&buf, end, clk, spec))
 		return buf;
@@ -1785,7 +1800,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 string_nocheck(buf, end, "(%pC?)", spec);
+		return error_string(buf, end, "(%pC?)", spec);
 #endif
 	}
 }
@@ -1843,7 +1858,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
 		names = gfpflag_names;
 		break;
 	default:
-		return string_nocheck(buf, end, "(%pG?)", spec);
+		return error_string(buf, end, "(%pG?)", spec);
 	}
 
 	return format_flags(buf, end, flags, names);
@@ -1899,7 +1914,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	str_spec.field_width = -1;
 
 	if (!IS_ENABLED(CONFIG_OF))
-		return string_nocheck(buf, end, "(%pOF?)", spec);
+		return error_string(buf, end, "(%pOF?)", spec);
 
 	if (check_pointer(&buf, end, dn, spec))
 		return buf;
@@ -1978,7 +1993,7 @@ static char *kobject_string(char *buf, char *end, void *ptr,
 		return device_node_string(buf, end, ptr, spec, fmt + 1);
 	}
 
-	return string_nocheck(buf, end, "(%pO?)", spec);
+	return error_string(buf, end, "(%pO?)", spec);
 }
 
 /*
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0
  2019-04-17 11:53 ` [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
@ 2019-04-17 13:44   ` Steven Rostedt
  0 siblings, 0 replies; 22+ messages in thread
From: Steven Rostedt @ 2019-04-17 13:44 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Sergey Senozhatsky, linux-kernel, Kees Cook

On Wed, 17 Apr 2019 13:53:42 +0200
Petr Mladek <pmladek@suse.com> wrote:

> restricted_pointer() pretends that it prints the address when kptr_restrict
> is set to zero. But it is never called in this situation. Instead,
> pointer() falls back to ptr_to_id() and hashes the pointer.
> 
> This patch removes the potential confusion. klp_restrict is checked only
> in restricted_pointer().
> 
> It actually fixes a small race when the address might get printed unhashed:
> 
> CPU0                            CPU1
> 
> pointer()
>   if (!kptr_restrict)
>      /* for example set to 2 */
>   restricted_pointer()
> 				/* echo 0 >/proc/sys/kernel/kptr_restrict */
> 				proc_dointvec_minmax_sysadmin()
> 				  klpr_restrict = 0;
>     switch(kptr_restrict)
>       case 0:
> 	break:
> 
>     number()
> 
> Fixes: commit ef0010a30935de4e0211 ("vsprintf: don't use 'restricted_pointer()' when not restricting")
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin Harding <me@tobin.cc>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> ---
>  lib/vsprintf.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index eb7b4a06e1f0..2af48948a973 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -725,8 +725,8 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
>  {
>  	switch (kptr_restrict) {
>  	case 0:
> -		/* Always print %pK values */
> -		break;
> +		/* Handle as %p, hash and do _not_ leak addresses. */
> +		return ptr_to_id(buf, end, ptr, spec);
>  	case 1: {
>  		const struct cred *cred;
>  
> @@ -2041,8 +2041,6 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>  			return buf;
>  		}
>  	case 'K':
> -		if (!kptr_restrict)
> -			break;
>  		return restricted_pointer(buf, end, ptr, spec);
>  	case 'N':
>  		return netdev_bits(buf, end, ptr, spec, fmt);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
@ 2019-04-18 14:34   ` Sergey Senozhatsky
  2019-04-18 14:43   ` Sergey Senozhatsky
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2019-04-18 14:34 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	linux-kernel

On (04/17/19 13:53), Petr Mladek wrote:
> 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.

Hmm, seems that error string now sometimes try to `guess' what was the
error, but the guess can be misleading.

A very small example.
flags_string() can have a number of fmt specifiers - p, v, g.

        switch (fmt[1]) {
        case 'p':
                flags = *(unsigned long *)flags_ptr;
                /* Remove zone id */
                flags &= (1UL << NR_PAGEFLAGS) - 1;
                names = pageflag_names;
                break;
        case 'v':
                flags = *(unsigned long *)flags_ptr;
                names = vmaflag_names;
                break;
        case 'g':
                flags = *(gfp_t *)flags_ptr;
                names = gfpflag_names;
                break;
        default:
                WARN_ONCE(1, "Unsupported flags modifier: %c\n", fmt[1]);
                return buf;
        }

The new error message, however, will hint '%pG', which may or may not
be helpful.

> -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;
> @@ -1760,8 +1767,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 string_nocheck(buf, end, "(%pG?)", spec);
>  	}

Wouldn't it be better to use fmt[1] instead?

	-ss

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
  2019-04-18 14:34   ` Sergey Senozhatsky
@ 2019-04-18 14:43   ` Sergey Senozhatsky
  2019-04-18 14:50   ` Sergey Senozhatsky
  2019-06-25 10:59   ` Geert Uytterhoeven
  3 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2019-04-18 14:43 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	linux-kernel

On (04/17/19 13:53), Petr Mladek wrote:
[..]
>  char *ip_addr_string(char *buf, char *end, const void *ptr,
>  		     struct printf_spec spec, const char *fmt)
>  {
> +	char *err_fmt_msg;
> +
>  	switch (fmt[1]) {
>  	case '6':
>  		return ip6_addr_string(buf, end, ptr, spec, fmt);
> @@ -1457,7 +1459,8 @@ char *ip_addr_string(char *buf, char *end, const void *ptr,
>  		}}
>  	}
>  
> -	return ptr_to_id(buf, end, ptr, spec);
> +	err_fmt_msg = fmt[0] == 'i' ? "(%pi?)" : "(%pI?)";
> +	return string_nocheck(buf, end, err_fmt_msg, spec);
>  }

We cannot err on fmt[0] here, can we. Both %pi and %pI are OK
and handled by pointer():

	switch (fmt[0]) {
	case 'I':
	case 'i':
		return ip_addr_string(buf, end, ptr, spec, fmt);


It's fmt[1] which can be unrecognized:

	switch (fmt[1]) {
	case '6':
	case '4':
	case 'S':
	}

	-ss

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
  2019-04-18 14:34   ` Sergey Senozhatsky
  2019-04-18 14:43   ` Sergey Senozhatsky
@ 2019-04-18 14:50   ` Sergey Senozhatsky
  2019-06-25 10:59   ` Geert Uytterhoeven
  3 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2019-04-18 14:50 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	linux-kernel

On (04/17/19 13:53), Petr Mladek wrote:
> 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.

Ah, I misunderstood the meaning of the question mark. The question
mark here is not a question, but a `wildcard' symbol which is replacing
a missing or unrecognized symbol.

	-ss

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling
  2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
                   ` (9 preceding siblings ...)
  2019-04-17 11:53 ` [PATCH v7 10/10] vsprintf: Limit the length of inlined error messages Petr Mladek
@ 2019-04-19  1:51 ` Sergey Senozhatsky
  2019-04-24 13:53   ` Petr Mladek
  10 siblings, 1 reply; 22+ messages in thread
From: Sergey Senozhatsky @ 2019-04-19  1:51 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	linux-kernel

On (04/17/19 13:53), Petr Mladek wrote:
> Crash in vsprintf() might be silent when it happens under logbuf_lock
> in vprintk_emit(). This patch set prevents most of the crashes by probing
> the address. The check is done only by %s and some %p* specifiers that need
> to dereference the address.
>
> Only the first byte of the address is checked to keep it simple. It should
> be enough to catch most problems.
>
> The check is explicitly done in each function that does the dereference.
> It helps to avoid the questionable strchr() of affected specifiers. This
> change motivated me to do some preparation patches that consolidated
> the error handling and cleaned the code a bit.

The patch set looks OK to me.

I got confused by 'pC?' error string, but once you start looking
at it as a regex (? - zero or one occurrences) things look OK.
Regex in dmesg/serial output might be something very new to people,
stack traces, after all, is a rather common error reporting mechanism.
So the previous "WARN_ON() + exact unrecognized fmt[N] char" was not
totally awful or wrong (well, it was, before we introduced printk_safe()),
but I don't have strong objections against that new regex thing.

FWIW,
Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling
  2019-04-19  1:51 ` [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Sergey Senozhatsky
@ 2019-04-24 13:53   ` Petr Mladek
  2019-04-26 13:02     ` Andy Shevchenko
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Mladek @ 2019-04-24 13:53 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, linux-kernel

On Fri 2019-04-19 10:51:12, Sergey Senozhatsky wrote:
> On (04/17/19 13:53), Petr Mladek wrote:
> > Crash in vsprintf() might be silent when it happens under logbuf_lock
> > in vprintk_emit(). This patch set prevents most of the crashes by probing
> > the address. The check is done only by %s and some %p* specifiers that need
> > to dereference the address.
> >
> > Only the first byte of the address is checked to keep it simple. It should
> > be enough to catch most problems.
> >
> > The check is explicitly done in each function that does the dereference.
> > It helps to avoid the questionable strchr() of affected specifiers. This
> > change motivated me to do some preparation patches that consolidated
> > the error handling and cleaned the code a bit.
> 
> The patch set looks OK to me.
> 
> I got confused by 'pC?' error string, but once you start looking
> at it as a regex (? - zero or one occurrences) things look OK.
> Regex in dmesg/serial output might be something very new to people,
> stack traces, after all, is a rather common error reporting mechanism.
> So the previous "WARN_ON() + exact unrecognized fmt[N] char" was not
> totally awful or wrong (well, it was, before we introduced printk_safe()),
> but I don't have strong objections against that new regex thing.
> 
> FWIW,
> Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Thanks a lot for review.

I have pushed the entire patchset into printk.git,
branch for-5.2-vsprintf-hardening to get some
test coverage via linux-next.

I still expect some feedback, especially from Andy
who seems to have a vacation these days.
I think that Andy wanted these changes rather sooner
than later, so I hope that he would be fine with it.
I could take it back in case of disagreement.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling
  2019-04-24 13:53   ` Petr Mladek
@ 2019-04-26 13:02     ` Andy Shevchenko
  2019-04-26 14:27       ` Petr Mladek
  0 siblings, 1 reply; 22+ messages in thread
From: Andy Shevchenko @ 2019-04-26 13:02 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, linux-kernel

On Wed, Apr 24, 2019 at 03:53:06PM +0200, Petr Mladek wrote:
> On Fri 2019-04-19 10:51:12, Sergey Senozhatsky wrote:
> > On (04/17/19 13:53), Petr Mladek wrote:
> > > Crash in vsprintf() might be silent when it happens under logbuf_lock
> > > in vprintk_emit(). This patch set prevents most of the crashes by probing
> > > the address. The check is done only by %s and some %p* specifiers that need
> > > to dereference the address.
> > >
> > > Only the first byte of the address is checked to keep it simple. It should
> > > be enough to catch most problems.
> > >
> > > The check is explicitly done in each function that does the dereference.
> > > It helps to avoid the questionable strchr() of affected specifiers. This
> > > change motivated me to do some preparation patches that consolidated
> > > the error handling and cleaned the code a bit.
> > 
> > The patch set looks OK to me.
> > 
> > I got confused by 'pC?' error string, but once you start looking
> > at it as a regex (? - zero or one occurrences) things look OK.
> > Regex in dmesg/serial output might be something very new to people,
> > stack traces, after all, is a rather common error reporting mechanism.
> > So the previous "WARN_ON() + exact unrecognized fmt[N] char" was not
> > totally awful or wrong (well, it was, before we introduced printk_safe()),
> > but I don't have strong objections against that new regex thing.
> > 
> > FWIW,
> > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> Thanks a lot for review.
> 
> I have pushed the entire patchset into printk.git,
> branch for-5.2-vsprintf-hardening to get some
> test coverage via linux-next.
> 
> I still expect some feedback, especially from Andy
> who seems to have a vacation these days.
> I think that Andy wanted these changes rather sooner
> than later, so I hope that he would be fine with it.
> I could take it back in case of disagreement.

They are good enough to me, thanks!
FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling
  2019-04-26 13:02     ` Andy Shevchenko
@ 2019-04-26 14:27       ` Petr Mladek
  0 siblings, 0 replies; 22+ messages in thread
From: Petr Mladek @ 2019-04-26 14:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sergey Senozhatsky, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, linux-kernel

On Fri 2019-04-26 16:02:04, Andy Shevchenko wrote:
> On Wed, Apr 24, 2019 at 03:53:06PM +0200, Petr Mladek wrote:
> > On Fri 2019-04-19 10:51:12, Sergey Senozhatsky wrote:
> > > On (04/17/19 13:53), Petr Mladek wrote:
> > > > Crash in vsprintf() might be silent when it happens under logbuf_lock
> > > > in vprintk_emit(). This patch set prevents most of the crashes by probing
> > > > the address. The check is done only by %s and some %p* specifiers that need
> > > > to dereference the address.
> > > >
> > > > Only the first byte of the address is checked to keep it simple. It should
> > > > be enough to catch most problems.
> > > >
> > > > The check is explicitly done in each function that does the dereference.
> > > > It helps to avoid the questionable strchr() of affected specifiers. This
> > > > change motivated me to do some preparation patches that consolidated
> > > > the error handling and cleaned the code a bit.
> > > 
> > > The patch set looks OK to me.
> > > 
> > > I got confused by 'pC?' error string, but once you start looking
> > > at it as a regex (? - zero or one occurrences) things look OK.
> > > Regex in dmesg/serial output might be something very new to people,
> > > stack traces, after all, is a rather common error reporting mechanism.
> > > So the previous "WARN_ON() + exact unrecognized fmt[N] char" was not
> > > totally awful or wrong (well, it was, before we introduced printk_safe()),
> > > but I don't have strong objections against that new regex thing.
> > > 
> > > FWIW,
> > > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > 
> > Thanks a lot for review.
> > 
> > I have pushed the entire patchset into printk.git,
> > branch for-5.2-vsprintf-hardening to get some
> > test coverage via linux-next.
> > 
> > I still expect some feedback, especially from Andy
> > who seems to have a vacation these days.
> > I think that Andy wanted these changes rather sooner
> > than later, so I hope that he would be fine with it.
> > I could take it back in case of disagreement.
> 
> They are good enough to me, thanks!
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks a lot. I have added the tag to the commits
in printk.git, branch for-5.2-vsprintf-hardening.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
                     ` (2 preceding siblings ...)
  2019-04-18 14:50   ` Sergey Senozhatsky
@ 2019-06-25 10:59   ` Geert Uytterhoeven
  2019-06-26 10:46     ` Petr Mladek
  3 siblings, 1 reply; 22+ messages in thread
From: Geert Uytterhoeven @ 2019-06-25 10:59 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	Linux Kernel Mailing List

Hi Petr,

On Wed, Apr 17, 2019 at 1:56 PM Petr Mladek <pmladek@suse.com> wrote:
> 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 inconsistent and not helpful.
> Using WARN() looks like an overkill for this type of error. pr_warn()
> is not good either. It would by handled via printk_safe 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.
>
> Note that it introduces a warning about that test_hashed() function
> is unused. It is going to be used again by a later patch.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1697,7 +1700,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 string_nocheck(buf, end, "(%pC?)", spec);

This one is OK, as there is no clock support compiled in.

> +
> +       if (!clk)
>                 return string(buf, end, NULL, spec);
>
>         switch (fmt[1]) {
> @@ -1706,7 +1712,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 ptr_to_id(buf, end, clk, spec);
> +               return string_nocheck(buf, end, "(%pC?)", spec);

What's the reason behind this change? This is not an error case,
but for printing the clock pointer as a distinguishable ID when using
the legacy clock framework, which does not store names with clocks.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-06-25 10:59   ` Geert Uytterhoeven
@ 2019-06-26 10:46     ` Petr Mladek
  2019-06-26 11:16       ` Geert Uytterhoeven
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Mladek @ 2019-06-26 10:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	Linux Kernel Mailing List

On Tue 2019-06-25 12:59:57, Geert Uytterhoeven wrote:
> Hi Petr,
> 
> On Wed, Apr 17, 2019 at 1:56 PM Petr Mladek <pmladek@suse.com> wrote:
> > 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 inconsistent and not helpful.
> > Using WARN() looks like an overkill for this type of error. pr_warn()
> > is not good either. It would by handled via printk_safe 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.
> >
> > Note that it introduces a warning about that test_hashed() function
> > is unused. It is going to be used again by a later patch.
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> 
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1706,7 +1712,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 ptr_to_id(buf, end, clk, spec);
> > +               return string_nocheck(buf, end, "(%pC?)", spec);
> 
> What's the reason behind this change? This is not an error case,
> but for printing the clock pointer as a distinguishable ID when using
> the legacy clock framework, which does not store names with clocks.

You are right. We should put back ptr_to_id() there.

Would you like to send a patch?

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers
  2019-06-26 10:46     ` Petr Mladek
@ 2019-06-26 11:16       ` Geert Uytterhoeven
  0 siblings, 0 replies; 22+ messages in thread
From: Geert Uytterhoeven @ 2019-06-26 11:16 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, Linus Torvalds,
	Tobin C . Harding, Joe Perches, Andrew Morton, Michal Hocko,
	Sergey Senozhatsky, Steven Rostedt, Sergey Senozhatsky,
	Linux Kernel Mailing List

Hi Petr,

On Wed, Jun 26, 2019 at 12:46 PM Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2019-06-25 12:59:57, Geert Uytterhoeven wrote:
> > On Wed, Apr 17, 2019 at 1:56 PM Petr Mladek <pmladek@suse.com> wrote:
> > > 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 inconsistent and not helpful.
> > > Using WARN() looks like an overkill for this type of error. pr_warn()
> > > is not good either. It would by handled via printk_safe 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.
> > >
> > > Note that it introduces a warning about that test_hashed() function
> > > is unused. It is going to be used again by a later patch.
> > >
> > > Signed-off-by: Petr Mladek <pmladek@suse.com>
> >
> > > --- a/lib/vsprintf.c
> > > +++ b/lib/vsprintf.c
> > > @@ -1706,7 +1712,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 ptr_to_id(buf, end, clk, spec);
> > > +               return string_nocheck(buf, end, "(%pC?)", spec);
> >
> > What's the reason behind this change? This is not an error case,
> > but for printing the clock pointer as a distinguishable ID when using
> > the legacy clock framework, which does not store names with clocks.
>
> You are right. We should put back ptr_to_id() there.

Thanks for the confirmation!

> Would you like to send a patch?

Sure, will do.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-06-26 11:16 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 11:53 [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
2019-04-17 11:53 ` [PATCH v7 01/10] vsprintf: Shuffle restricted_pointer() Petr Mladek
2019-04-17 11:53 ` [PATCH v7 02/10] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
2019-04-17 13:44   ` Steven Rostedt
2019-04-17 11:53 ` [PATCH v7 03/10] vsprintf: Do not check address of well-known strings Petr Mladek
2019-04-17 11:53 ` [PATCH v7 04/10] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
2019-04-17 11:53 ` [PATCH v7 05/10] vsprintf: Factor out %pV handler as va_format() Petr Mladek
2019-04-17 11:53 ` [PATCH v7 06/10] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
2019-04-17 11:53 ` [PATCH v7 07/10] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
2019-04-18 14:34   ` Sergey Senozhatsky
2019-04-18 14:43   ` Sergey Senozhatsky
2019-04-18 14:50   ` Sergey Senozhatsky
2019-06-25 10:59   ` Geert Uytterhoeven
2019-06-26 10:46     ` Petr Mladek
2019-06-26 11:16       ` Geert Uytterhoeven
2019-04-17 11:53 ` [PATCH v7 08/10] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
2019-04-17 11:53 ` [PATCH v7 09/10] vsprintf: Avoid confusion between invalid address and value Petr Mladek
2019-04-17 11:53 ` [PATCH v7 10/10] vsprintf: Limit the length of inlined error messages Petr Mladek
2019-04-19  1:51 ` [PATCH v7 00/10] vsprintf: Prevent silent crashes and consolidate error handling Sergey Senozhatsky
2019-04-24 13:53   ` Petr Mladek
2019-04-26 13:02     ` Andy Shevchenko
2019-04-26 14:27       ` Petr Mladek

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).