[RFC] vsprintf: Prevent crash when dereferencing invalid pointers

From: Petr Mladek
Date: Wed Mar 07 2018 - 10:27:24 EST


We already prevent crash when derefencing some obviously broken
pointers. 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() formats the string 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.

In general, we should do our best to get useful message from printk().
This patch tries to find a wide range of invalid strings using
probe_kernel_read(). Also it makes the handling unified. We print:

+ (null) only when pure NULL pointer is dereferenced
+ (efault) in all other cases

Note that we could not print the exact pointer value from security reasons.
Developers need print the pointer using %px to get the real value.

Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
---
lib/vsprintf.c | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d7a708f82559..c1d8de016081 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -520,6 +520,19 @@ char *number(char *buf, char *end, unsigned long long num,
return buf;
}

+static const char *check_pointer_access(const void *ptr)
+{
+ unsigned char byte;
+
+ if (!ptr)
+ return "(null)";
+
+ if (probe_kernel_read(&byte, ptr, 1))
+ return "(efault)";
+
+ return 0;
+}
+
static noinline_for_stack
char *special_hex_number(char *buf, char *end, unsigned long long num, int size)
{
@@ -586,9 +599,11 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
{
int len = 0;
size_t lim = spec.precision;
+ const char *err_msg;

- if ((unsigned long)s < PAGE_SIZE)
- s = "(null)";
+ err_msg = check_pointer_access(s);
+ if (err_msg)
+ s = err_msg;

while (lim--) {
char c = *s++;
@@ -1848,15 +1863,19 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
struct printf_spec spec)
{
const int default_width = 2 * sizeof(void *);
+ const char *err_msg = NULL;
+
+ if (*fmt != 'K' && *fmt != 'x')
+ err_msg = check_pointer_access(ptr);

- if (!ptr && *fmt != 'K' && *fmt != 'x') {
+ if (err_msg) {
/*
- * Print (null) with the same width as a pointer so it makes
- * tabular output look nice.
+ * Print the error message 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(buf, end, "(null)", spec);
+ return string(buf, end, err_msg, spec);
}

switch (*fmt) {
@@ -2571,11 +2590,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_access(save_str);
+ if (err_msg)
+ save_str = err_msg;
+
len = strlen(save_str) + 1;
if (str + len < end)
memcpy(str, save_str, len);
--
2.13.6