Re: [PATCH] vsprintf: Do not break early boot with probing addresses

From
Petr Mladek <pmladek@suse.com>
Date
2019-05-13 12:42:25
Also in
linux-arch, linux-s390, lkml
Reply to
Andy Shevchenko, 2019-05-13
On Mon 2019-05-13 12:13:20, Andy Shevchenko wrote:
On Mon, May 13, 2019 at 08:52:41AM +0000, David Laight wrote:
quoted
From: christophe leroy
quoted
Sent: 10 May 2019 18:35
Le 10/05/2019 à 18:24, Steven Rostedt a écrit :
quoted
On Fri, 10 May 2019 10:42:13 +0200
Petr Mladek [off-list ref] wrote:
quoted
quoted
quoted
quoted
-	if (probe_kernel_address(ptr, byte))
+	if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
  		return "(efault)";
"efault" looks a bit like a spellling mistake for "default".
It's a special, thus it's in parenthesis, though somebody can be
misguided.
quoted
quoted
Usually, < PAGE_SIZE means NULL pointer dereference (via the member of a
struct)
Maybe the caller should pass in a short buffer so that you can return
"(err-%d)"
or "(null+%#x)" ?
There are many vsprintf() users. I am afraid that nobody would want
to define extra buffers for error messages. It must fit into
the one for the formatted string.

In both cases it should be limited to the size of pointer (8 or 16
characters). Something like "(e:%4d)" would work for error codes.
I am afraid that we could get 10 different proposals from a huge
enough group of people. I wanted to start with something simple
(source code). I know that (efault) might be confused with
"default" but it is still just one string to grep.

The "(null)" is good enough by itself and already an established
practice..
(efault) made more sense with the probe_kernel_read() that
checked wide range of addresses. Well, I still think that
it makes sense to distinguish a pure NULL. And it still
used also for IS_ERR_VALUE().

Best Regards,
Petr
Thread (39 messages) 39 messages, 11 authors, last reply 2019-05-15
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help