Messages in this thread | | | Subject | Re: [PATCH v5 09/11] vsprintf: Prevent crash when dereferencing invalid pointers | From | Andy Shevchenko <> | Date | Wed, 25 Apr 2018 18:10:54 +0300 |
| |
On Wed, 2018-04-25 at 13:12 +0200, Petr Mladek wrote: > 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.
> +static const char *check_pointer_access(const void *ptr) > +{ > + char byte; > + > + if (!ptr) > + return "(null)"; > + > + if (probe_kernel_address(ptr, byte)) > + return "(efault)"; > + > + return NULL; > +} > + > +static bool valid_pointer_access(char **buf, char *end, const void > *ptr, > + struct printf_spec spec) > +{ > + const char *err_msg; > + > + err_msg = check_pointer_access(ptr); > + if (err_msg) { > + *buf = valid_string(*buf, end, err_msg, spec); > + return false; > + } > + > + return true; > +}
I would preserve similar style of buf pointer handling, i.e.
static char *valid_pointer_access(char **buf, char *end, const void *ptr, struct printf_spec spec) { const char *err_msg;
err_msg = check_pointer_access(ptr); if (err_msg) return = valid_string(*buf, end, err_msg, spec);
return NULL; }
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |