Re: [PATCH v3 1/1] vsprintf: protect kernel from panic due to non-canonical pointer dereference

[Andy Shevchenko]

On Tue, Oct 25, 2022 at 10:40:37AM +0200, Petr Mladek wrote:
> On Thu 2022-10-20 19:03:23, Andy Shevchenko wrote:
> > On Thu, Oct 20, 2022 at 10:52:03AM -0400, Konrad Rzeszutek Wilk wrote:

...

> > OK, let's assume user recognizes this as a bug, what should they do in order
> > to provide a better description of the bug, so developer can easily debug
> > and fix it?
> 
> WARN() would provide similar information as panic() without actually
> crashing the kernel.

Unless one provides panic_on_warn (or how is it called?).

> > > Would we not want that experience for users ?
> > 
> > Yes, if it is a bug in the kernel we want to know it with all possible details.
> > Hiding bugs is a way to nowhere.
> 
> I agree but we should always distinguish between fatal problems where
> the system could hardly continue working and unexpected behavior that
> is not critical.
> 
> Many error code paths handle unexpected situations. Some problems are
> caused by users and some by bugs in the code. The kernel could always
> refuse doing some operation rather than crash. People will report
> it because it does not work. And there are non-destructive ways how
> to show useful debugging information.

Initially, if I understand correctly, the idea of that check was exactly to
guard against special pointers (NULL and error). Now this is getting wider
and I'm not sure hiding a crash is good thing to go.

Hypothetical situation: the "invalid" pointer is just one that gets LSB
shuffled a bit (some of the frameworks use lower bits to keep some information
there). That said, kernel is not going to crash elsewhere. How user will know
that unmasked pointer went to the printf()?

I honestly think that this or similar change will bring more harm than help.

-- 
With Best Regards,
Andy Shevchenko