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

2019-05-15 Thread David Laight
From: Petr Mladek > Sent: 15 May 2019 08:36 > On Tue 2019-05-14 14:37:51, Steven Rostedt wrote: > > > > [ Purple is a nice shade on the bike shed. ;-) ] > > > > On Tue, 14 May 2019 11:02:17 +0200 > > Geert Uytterhoeven wrote: > > > > > On Tue, May 14, 2019 at 10:29 AM David Laight > > > wrote:

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

2019-05-15 Thread Petr Mladek
On Wed 2019-05-15 09:23:05, Geert Uytterhoeven wrote: > Hi Steve, > > On Tue, May 14, 2019 at 9:35 PM Steven Rostedt wrote: > > On Tue, 14 May 2019 21:13:06 +0200 > > Geert Uytterhoeven wrote: > > > > > Do we care about the value? "(-E%u)"? > > > > > > > > That too could be confusing. What

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

2019-05-15 Thread Petr Mladek
On Tue 2019-05-14 14:37:51, Steven Rostedt wrote: > > [ Purple is a nice shade on the bike shed. ;-) ] > > On Tue, 14 May 2019 11:02:17 +0200 > Geert Uytterhoeven wrote: > > > On Tue, May 14, 2019 at 10:29 AM David Laight > > wrote: > > > > And I like Steven's "(fault)" idea. > > > > How

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

2019-05-15 Thread Geert Uytterhoeven
Hi Steve, On Tue, May 14, 2019 at 9:35 PM Steven Rostedt wrote: > On Tue, 14 May 2019 21:13:06 +0200 > Geert Uytterhoeven wrote: > > > > Do we care about the value? "(-E%u)"? > > > > > > That too could be confusing. What would (-E22) be considered by a user > > > doing an sprintf() on some

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

2019-05-15 Thread Sergey Senozhatsky
On (05/14/19 21:13), Geert Uytterhoeven wrote: > I would immediately understand there's a missing IS_ERR() check in a > function that can return -EINVAL, without having to add a new printk() > to find out what kind of bogus value has been received, and without > having to reboot, and trying to

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

2019-05-14 Thread Steven Rostedt
On Tue, 14 May 2019 21:13:06 +0200 Geert Uytterhoeven wrote: > > > Do we care about the value? "(-E%u)"? > > > > That too could be confusing. What would (-E22) be considered by a user > > doing an sprintf() on some string. I know that would confuse me, or I > > would think that it was what the

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

2019-05-14 Thread Geert Uytterhoeven
Hi Steve, On Tue, May 14, 2019 at 8:37 PM Steven Rostedt wrote: > On Tue, 14 May 2019 11:02:17 +0200 > Geert Uytterhoeven wrote: > > On Tue, May 14, 2019 at 10:29 AM David Laight > > wrote: > > > > And I like Steven's "(fault)" idea. > > > > How about this: > > > > > > > > if ptr <

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

2019-05-14 Thread Steven Rostedt
[ Purple is a nice shade on the bike shed. ;-) ] On Tue, 14 May 2019 11:02:17 +0200 Geert Uytterhoeven wrote: > On Tue, May 14, 2019 at 10:29 AM David Laight wrote: > > > And I like Steven's "(fault)" idea. > > > How about this: > > > > > > if ptr < PAGE_SIZE -> "(null)" >

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

2019-05-14 Thread Geert Uytterhoeven
On Tue, May 14, 2019 at 10:29 AM David Laight wrote: > > And I like Steven's "(fault)" idea. > > How about this: > > > > if ptr < PAGE_SIZE -> "(null)" > > if IS_ERR_VALUE(ptr)-> "(fault)" > > > > -ss > > Or: > if (ptr < PAGE_SIZE) >

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

2019-05-14 Thread David Laight
> And I like Steven's "(fault)" idea. > How about this: > > if ptr < PAGE_SIZE -> "(null)" > if IS_ERR_VALUE(ptr)-> "(fault)" > > -ss Or: if (ptr < PAGE_SIZE) return ptr ? "(null+)" : "(null)"; if IS_ERR_VALUE(ptr)

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

2019-05-13 Thread Sergey Senozhatsky
On (05/14/19 11:07), Sergey Senozhatsky wrote: > How about this: > > if ptr < PAGE_SIZE -> "(null)" No, this is totally stupid. Forget about it. Sorry. > if IS_ERR_VALUE(ptr)-> "(fault)" But Steven's "(fault)" is nice. -ss

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

2019-05-13 Thread Sergey Senozhatsky
On (05/13/19 14:42), Petr Mladek wrote: > > 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

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

2019-05-13 Thread Steven Rostedt
On Mon, 13 May 2019 14:42:20 +0200 Petr Mladek wrote: > > 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

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

2019-05-13 Thread Petr Mladek
On Mon 2019-05-13 12:13:20, Andy Shevchenko wrote: > On Mon, May 13, 2019 at 08:52:41AM +, David Laight wrote: > > From: christophe leroy > > > Sent: 10 May 2019 18:35 > > > Le 10/05/2019 à 18:24, Steven Rostedt a écrit : > > > > On Fri, 10 May 2019 10:42:13 +0200 > > > > Petr Mladek wrote: >

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

2019-05-13 Thread Petr Mladek
On Fri 2019-05-10 12:40:58, Steven Rostedt wrote: > On Fri, 10 May 2019 18:32:58 +0200 > Martin Schwidefsky wrote: > > > On Fri, 10 May 2019 12:24:01 -0400 > > Steven Rostedt wrote: > > > > > On Fri, 10 May 2019 10:42:13 +0200 > > > Petr Mladek wrote: > > > > > > > static const char

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

2019-05-13 Thread Andy Shevchenko
On Mon, May 13, 2019 at 08:52:41AM +, David Laight wrote: > From: christophe leroy > > Sent: 10 May 2019 18:35 > > Le 10/05/2019 à 18:24, Steven Rostedt a écrit : > > > On Fri, 10 May 2019 10:42:13 +0200 > > > Petr Mladek wrote: > > >> -if (probe_kernel_address(ptr, byte)) > > >> +

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

2019-05-13 Thread David Laight
From: christophe leroy > Sent: 10 May 2019 18:35 > Le 10/05/2019 à 18:24, Steven Rostedt a écrit : > > On Fri, 10 May 2019 10:42:13 +0200 > > Petr Mladek wrote: > > > >> static const char *check_pointer_msg(const void *ptr) > >> { > >> - char byte; > >> - > >>if (!ptr) > >>

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

2019-05-10 Thread christophe leroy
Le 10/05/2019 à 18:24, Steven Rostedt a écrit : On Fri, 10 May 2019 10:42:13 +0200 Petr Mladek wrote: static const char *check_pointer_msg(const void *ptr) { - char byte; - if (!ptr) return "(null)"; - if (probe_kernel_address(ptr, byte)) + if

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

2019-05-10 Thread Martin Schwidefsky
On Fri, 10 May 2019 12:40:58 -0400 Steven Rostedt wrote: > On Fri, 10 May 2019 18:32:58 +0200 > Martin Schwidefsky wrote: > > > On Fri, 10 May 2019 12:24:01 -0400 > > Steven Rostedt wrote: > > > > > On Fri, 10 May 2019 10:42:13 +0200 > > > Petr Mladek wrote: > > > > > > > static

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

2019-05-10 Thread Andy Shevchenko
On Fri, May 10, 2019 at 12:24:01PM -0400, Steven Rostedt wrote: > On Fri, 10 May 2019 10:42:13 +0200 > Petr Mladek wrote: > > > static const char *check_pointer_msg(const void *ptr) > > { > > - char byte; > > - > > if (!ptr) > > return "(null)"; > > > > - if

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

2019-05-10 Thread Steven Rostedt
On Fri, 10 May 2019 18:32:58 +0200 Martin Schwidefsky wrote: > On Fri, 10 May 2019 12:24:01 -0400 > Steven Rostedt wrote: > > > On Fri, 10 May 2019 10:42:13 +0200 > > Petr Mladek wrote: > > > > > static const char *check_pointer_msg(const void *ptr) > > > { > > > - char byte; > > > - > >

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

2019-05-10 Thread Martin Schwidefsky
On Fri, 10 May 2019 12:24:01 -0400 Steven Rostedt wrote: > On Fri, 10 May 2019 10:42:13 +0200 > Petr Mladek wrote: > > > static const char *check_pointer_msg(const void *ptr) > > { > > - char byte; > > - > > if (!ptr) > > return "(null)"; > > > > - if

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

2019-05-10 Thread Steven Rostedt
On Fri, 10 May 2019 10:42:13 +0200 Petr Mladek wrote: > static const char *check_pointer_msg(const void *ptr) > { > - char byte; > - > if (!ptr) > return "(null)"; > > - if (probe_kernel_address(ptr, byte)) > + if ((unsigned long)ptr < PAGE_SIZE ||

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

2019-05-10 Thread Petr Mladek
On Fri 2019-05-10 10:42:13, Petr Mladek wrote: > The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing > invalid pointers") broke boot on several architectures. The common > pattern is that probe_kernel_read() is not working during early > boot because userspace access

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

2019-05-10 Thread Michael Ellerman
David Laight writes: > From: Michal Suchánek >> Sent: 09 May 2019 14:38 > ... >> > The problem is the combination of some new code called via printk(), >> > check_pointer() which calls probe_kernel_read(). That then calls >> > allow_user_access() (PPC_KUAP) and that uses mmu_has_feature() too

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

2019-05-10 Thread Sergey Senozhatsky
On (05/10/19 10:42), Petr Mladek wrote: [..] > Fixes: 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing invalid > pointers") > Signed-off-by: Petr Mladek FWIW Reviewed-by: Sergey Senozhatsky -ss

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

2019-05-10 Thread Petr Mladek
The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing invalid pointers") broke boot on several architectures. The common pattern is that probe_kernel_read() is not working during early boot because userspace access framework is not ready. It is a generic problem. We have to

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

2019-05-10 Thread Sergey Senozhatsky
On (05/10/19 10:06), Petr Mladek wrote: [..] > I am going to send a patch replacing probe_kernel_address() with > a simple check: > > if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr)) > return "(efault)"; I'm OK with this. Probing ptrs was a good idea, it just didn't

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

2019-05-10 Thread Petr Mladek
On Fri 2019-05-10 14:07:09, Sergey Senozhatsky wrote: > On (05/09/19 21:47), Linus Torvalds wrote: > >[ Sorry about html and mobile crud, I'm not at the computer right now ] > >How about we just undo the whole misguided probe_kernel_address() thing? > > But the problem will remain -

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

2019-05-10 Thread Michael Ellerman
Sergey Senozhatsky writes: > On (05/09/19 21:47), Linus Torvalds wrote: >>[ Sorry about html and mobile crud, I'm not at the computer right now ] >>How about we just undo the whole misguided probe_kernel_address() thing? > > But the problem will remain - %pS/%pF on PPC (and some other

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

2019-05-09 Thread Linus Torvalds
[ Sorry about html and mobile crud, I'm not at the computer right now ] How about we just undo the whole misguided probe_kernel_address() thing? The excuse for is was that it would avoid crashes. It turns out that was wrong, and that it just made things much worse. Honestly, we haven't really

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

2019-05-09 Thread Sergey Senozhatsky
On (05/09/19 21:47), Linus Torvalds wrote: >[ Sorry about html and mobile crud, I'm not at the computer right now ] >How about we just undo the whole misguided probe_kernel_address() thing? But the problem will remain - %pS/%pF on PPC (and some other arch-s) do

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

2019-05-09 Thread Sergey Senozhatsky
On (05/09/19 14:19), Petr Mladek wrote: > 1. Report on Power: > > Kernel crashes very early during boot with with CONFIG_PPC_KUAP and > CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG > > The problem is the combination of some new code called via printk(), > check_pointer() which calls

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

2019-05-09 Thread David Laight
From: Michal Suchánek > Sent: 09 May 2019 14:38 ... > > The problem is the combination of some new code called via printk(), > > check_pointer() which calls probe_kernel_read(). That then calls > > allow_user_access() (PPC_KUAP) and that uses mmu_has_feature() too early > > (before we've patched

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

2019-05-09 Thread Petr Mladek
On Thu 2019-05-09 09:13:57, Steven Rostedt wrote: > On Thu, 9 May 2019 14:19:23 +0200 > Petr Mladek wrote: > > > The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing > > invalid pointers") broke boot on several architectures. The common > > pattern is that

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

2019-05-09 Thread Michal Suchánek
On Thu, 9 May 2019 14:19:23 +0200 Petr Mladek wrote: > The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing > invalid pointers") broke boot on several architectures. The common > pattern is that probe_kernel_read() is not working during early > boot because userspace access

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

2019-05-09 Thread Steven Rostedt
On Thu, 9 May 2019 14:19:23 +0200 Petr Mladek wrote: > The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing > invalid pointers") broke boot on several architectures. The common > pattern is that probe_kernel_read() is not working during early > boot because userspace access

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

2019-05-09 Thread Andy Shevchenko
On Thu, May 09, 2019 at 02:19:23PM +0200, Petr Mladek wrote: > The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing > invalid pointers") broke boot on several architectures. The common > pattern is that probe_kernel_read() is not working during early > boot because userspace

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

2019-05-09 Thread Petr Mladek
The commit 3e5903eb9cff70730 ("vsprintf: Prevent crash when dereferencing invalid pointers") broke boot on several architectures. The common pattern is that probe_kernel_read() is not working during early boot because userspace access framework is not ready. The check is only the best effort.