[RFC][PATCH 1/3] vsprintf: Prevent vbin_printf() from using dereferenced pointers

[Steven Rostedt]

From: "Steven Rostedt (Red Hat)" <rostedt@xxxxxxxxxxx>

vbin_printf() is used in conjunction with bstr_printf() to record some
printf() statment and to show it later. It is currently used exclusively
with trace_printk() which records the binary format into the trace ring
buffer and formats the output at a later time when the ring buffer is read.
This helps speed up the calculations of trace_printk() when it is called to
lower the impact the tracing has on the running system. The more execution
that can be delayed to the time of reading the trace, the less intrusive the
trace is on the running executiong being traced.

The problem arises with dereferenced pointers. The printk() format allows
several types of format fields outside of normal printf() that helps in
outputing various kernel structures. This includes dereferenced function
pointers (from powerpc), cpumasks, MAC addresses, ip addresses to name a
few. Currently the pointer to these structures are saved in the buffer by
vbin_printf() and then dereferenced with bstr_printf().

When vbin_printf() is called, it is in the context of where the pointer is
in use, with necessary locks and such. It can also be assumed that because
the pointer is being passed to the vbin_printf() from the user, that that
pointer currently exists. As bstr_printf() can be displayed at a later time,
this is not the case. Locks wont be held and the pointer may not even exist
anymore. This causes bstr_printf() to dereference a bad pointer and can
cause a kernel oops.

When should be done is that vbin_printf() should save the contents of what
the pointer points to and bstr_printf() should only output what it has in
the buffer, without the need to dereference. But in the mean time, a check
is now added to see if a pointer type is supported by vbin_printf() and if
it is not, then "Unsupported type:%pX" is output instead. This prevents the
bstr_printf() from doing a kernel oops. Luckily, trace_printk() is the only
user and that is only added for debugging the kernel.

Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
---
 lib/vsprintf.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0967771d8f7f..b2a331d948a2 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2257,6 +2257,62 @@ EXPORT_SYMBOL(sprintf);
  * bstr_printf() - Binary data to text string
  */
 
+/*
+ * As vbin_printf() is not the same as a normal printf(), as it is broken
+ * into two parts:
+ *   The recording of the pointer information.
+ *   The printing of the information.
+ * As the pointer may change, or even worse no longer exist, then dereferenced
+ * pointers must not be used with this utility unless it is supported.
+ */
+
+static bool supported_bin_ptr(const char *fmt)
+{
+	bool supported = false;
+
+	switch (fmt[0]) {
+	case 'F':
+	case 'f':
+		/* Some archs dereference function pointers */
+		if (vbin_printf == 
+		    dereference_function_descriptor(vbin_printf)) {
+			supported = true;
+			break;
+		}
+		/* Fall through */
+	case 'R':
+	case 'r':
+	case 'b':
+	case 'M':
+	case 'm':
+	case 'I':
+	case 'i':
+	case 'E':
+	case 'U':
+	case 'V':
+		break;
+	case 'N':
+		if (fmt[1] != 'F') {
+			supported = true;
+			break;
+		}
+		/* fall through */
+	case 'a':
+	case 'd':
+	case 'C':
+	case 'D':
+	case 'g':
+	case 'G':
+		break;
+	default:
+		supported = true;
+	}
+
+	return supported;
+}
+
+const static char unsupported_str[] = "Unsupported type:%p";
+
 /**
  * vbin_printf - Parse a format string and place args' binary value in a buffer
  * @bin_buf: The buffer to place args' binary value
@@ -2490,12 +2546,30 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 			break;
 		}
 
-		case FORMAT_TYPE_PTR:
-			str = pointer(fmt, str, end, get_arg(void *), spec);
+		case FORMAT_TYPE_PTR: {
+			const char *_fmt = fmt;
+
+			if (!supported_bin_ptr(fmt)) {
+				int len = sizeof(unsupported_str) + 1;
+
+				if (str + len <= end) {
+					strcpy(str, unsupported_str);
+					str += sizeof(unsupported_str) - 1;
+					str[0] = fmt[0];
+					str[1] = ':';
+					str += 2;
+				} else
+					str += len;
+				/* Just show the pointer itself */
+				_fmt = "";
+				spec.field_width = -1;
+			}
+			str = pointer(_fmt, str, end, get_arg(void *), spec);
+
 			while (isalnum(*fmt))
 				fmt++;
 			break;
-
+		}
 		case FORMAT_TYPE_PERCENT_CHAR:
 			if (str < end)
 				*str = '%';
-- 
2.8.1