LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Petr Mladek' <pmladek@suse.com>, Steven Rostedt <rostedt@goodmis.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	christophe leroy <christophe.leroy@c-s.fr>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Tobin C . Harding" <me@tobin.cc>, Michal Hocko <mhocko@suse.cz>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"Russell Currey" <ruscur@russell.cc>,
	Stephen Rothwell <sfr@ozlabs.org>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>
Subject: RE: [PATCH] vsprintf: Do not break early boot with probing addresses
Date: Wed, 15 May 2019 09:00:23 +0000	[thread overview]
Message-ID: <0a816ea272a0405f89d8ca7178604531@AcuMS.aculab.com> (raw)
In-Reply-To: <20190515073542.y6ru2nfagtcrpdl7@pathway.suse.cz>

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 <geert@linux-m68k.org> wrote:
> >
> > > On Tue, May 14, 2019 at 10:29 AM David Laight <David.Laight@aculab.com> 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)
> > > >                 return ptr ? "(null+)" : "(null)";
> >
> > Hmm, that is useful.
> >
> > > >         if IS_ERR_VALUE(ptr)
> > > >                 return "(errno)"
> >
> > I still prefer "(fault)" as is pretty much all I would expect from a
> > pointer dereference, even if it is just bad parsing of, say, a parsing
> > an MAC address. "fault" is generic enough. "errno" will be confusing,
> > because that's normally a variable not a output.
> >
> > >
> > > 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 %pX displayed, and wonder why it
> > displayed it that way. Whereas "(fault)" is quite obvious for any %p
> > use case.
> 
> This discussion clearly shows that it is hard to make anyone happy.
> 
> I considered switching to "(fault)" because there seems to be more
> people in favor of this.
> 
> But there is used also "(einval)" when an unsupported pointer
> modifier is passed. The idea is to show error codes that people
> are familiar with.
> 
> It might have been better to use the uppercase "(EFAULT)" and
> "(EINVAL)" to make it more obvious. But I wanted to follow
> the existing style with the lowercase "(null)".

Printing 'fault' when the code was (trying to) validate the
address was ok.
When the only check is for an -errno value it seems wrong as
most invalid addresses will actually fault (and panic).

The reason modern printf generate "(null)" is that it is far too
easy for a diagnostic print to fail to test a pointer.
It also makes it easier when 'throwing in' printf while debugging
to add a single trace that will work regardless of whether a
call had succeeded or not.

With the Linux kernel putting errno values into pointers it
seems likely that most invalid pointers in printf will actaully
be error values.
Printing the value will be helpful during debugging - as a
trace can be put after a call and show the parameters and result.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


      reply	other threads:[~2019-05-15  9:00 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-09 12:19 Petr Mladek
2019-05-09 13:05 ` Andy Shevchenko
2019-05-09 13:13 ` Steven Rostedt
2019-05-09 14:06   ` Petr Mladek
2019-05-09 13:38 ` Michal Suchánek
2019-05-09 13:46   ` David Laight
2019-05-10 10:21     ` Michael Ellerman
2019-05-10  4:32 ` Sergey Senozhatsky
     [not found]   ` <CAHk-=wiP+hwSqEW0dM6AYNWUR7jXDkeueq69et6ahfUgV7hC3w@mail.gmail.com>
2019-05-10  5:07     ` Sergey Senozhatsky
2019-05-10  6:41       ` Michael Ellerman
2019-05-10  8:06       ` Petr Mladek
2019-05-10  8:16         ` Sergey Senozhatsky
2019-05-10  8:42           ` Petr Mladek
2019-05-10  8:51             ` Sergey Senozhatsky
2019-05-10 14:49             ` Petr Mladek
2019-05-10 16:24             ` Steven Rostedt
2019-05-10 16:32               ` Martin Schwidefsky
2019-05-10 16:40                 ` Steven Rostedt
2019-05-10 16:45                   ` Martin Schwidefsky
2019-05-13 12:24                   ` Petr Mladek
2019-05-10 16:41               ` Andy Shevchenko
2019-05-10 17:35               ` christophe leroy
2019-05-13  8:52                 ` David Laight
2019-05-13  9:13                   ` Andy Shevchenko
2019-05-13 12:42                     ` Petr Mladek
2019-05-13 14:15                       ` Steven Rostedt
2019-05-14  2:07                       ` Sergey Senozhatsky
2019-05-14  2:25                         ` Sergey Senozhatsky
2019-05-14  8:28                         ` David Laight
2019-05-14  9:02                           ` Geert Uytterhoeven
2019-05-14 18:37                             ` Steven Rostedt
2019-05-14 19:13                               ` Geert Uytterhoeven
2019-05-14 19:35                                 ` Steven Rostedt
2019-05-15  7:23                                   ` Geert Uytterhoeven
2019-05-15  7:53                                     ` Petr Mladek
2019-05-15  6:21                                 ` Sergey Senozhatsky
2019-05-15  7:35                               ` Petr Mladek
2019-05-15  9:00                                 ` David Laight [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a816ea272a0405f89d8ca7178604531@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=christophe.leroy@c-s.fr \
    --cc=geert@linux-m68k.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=me@tobin.cc \
    --cc=mhocko@suse.cz \
    --cc=mpe@ellerman.id.au \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=ruscur@russell.cc \
    --cc=schwidefsky@de.ibm.com \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=sfr@ozlabs.org \
    --cc=torvalds@linux-foundation.org \
    --subject='RE: [PATCH] vsprintf: Do not break early boot with probing addresses' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).