From: Kent Overstreet
> Sent: 19 May 2022 18:24
>
> This implements two new format strings: both do the same thing, one more
> compatible with current gcc format string checking, the other that we'd
> like to standardize:
>
> %p(%p) - more compatible
> %(%p) - more prettier
>
> Both can take variable numbers of arguments, i.e. %(%p,%p,%p).
>
> They're used to indicate that snprintf or pr_buf should interpret the
> next argument as a pretty-printer function to call, and subsequent
> arguments within the parentheses should be passed to the pretty-printer.
I suspect this a very good way to blow the kernel stack.
The highest stack use is already very likely to be inside
the printf code in an error path somewhere.
...
> The goal is to replace most of our %p format extensions with this
> interface, and to move pretty-printers out of the core vsprintf.c code -
One advantage of the current scheme is that is reasonably safe
and easy to use.
Perhaps too many extra formats have been added recently.
This all seems like a recipe for disaster with functions being
called with the wrong number of parameters
(I can't think how you can compile-time check it).
Double copying using a temporary buffer isn't the end of the world.
It is only a problem because pr_cont() is basically impossible.
But since kernel printf ought to be formatted to reasonable
line length that isn't really an issue.
printf() is expensive an extra memory copy is probably noise.
...
> Currently, we can only call pretty printers with pointer arguments. This
> could be changed to also allow at least integer arguments in the future
> by using libffi.
I'm sure I remember something else trying to use that.
IIRC it is basically broken by design.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Thu, May 19, 2022 at 09:06:24PM +0000, David Laight wrote:
> From: Kent Overstreet
> > Sent: 19 May 2022 18:24
> >
> > This implements two new format strings: both do the same thing, one more
> > compatible with current gcc format string checking, the other that we'd
> > like to standardize:
> >
> > %p(%p) - more compatible
> > %(%p) - more prettier
> >
> > Both can take variable numbers of arguments, i.e. %(%p,%p,%p).
> >
> > They're used to indicate that snprintf or pr_buf should interpret the
> > next argument as a pretty-printer function to call, and subsequent
> > arguments within the parentheses should be passed to the pretty-printer.
>
> I suspect this a very good way to blow the kernel stack.
> The highest stack use is already very likely to be inside
> the printf code in an error path somewhere.
By getting rid of stack allocated buffers, I've been _reducing_ stack usage.
Also, the new printbuf calling convention reduces stack usage as well.
It's true that we'll want to keep the stack usage of pr_buf -> pretty printer ->
pr_buf again minimal, but I don't see any difficulties there the way the code is
structured now.
>
> ...
> > The goal is to replace most of our %p format extensions with this
> > interface, and to move pretty-printers out of the core vsprintf.c code -
>
> One advantage of the current scheme is that is reasonably safe
> and easy to use.
> Perhaps too many extra formats have been added recently.
> This all seems like a recipe for disaster with functions being
> called with the wrong number of parameters
> (I can't think how you can compile-time check it).
We can't check it at compile time yet, it's true - printf format checking will
need to be extended. But we're already talking about doing that.
> Double copying using a temporary buffer isn't the end of the world.
> It is only a problem because pr_cont() is basically impossible.
> But since kernel printf ought to be formatted to reasonable
> line length that isn't really an issue.
> printf() is expensive an extra memory copy is probably noise.
>
> ...
> > Currently, we can only call pretty printers with pointer arguments. This
> > could be changed to also allow at least integer arguments in the future
> > by using libffi.
>
> I'm sure I remember something else trying to use that.
> IIRC it is basically broken by design.
Hmm? libffi is the standard for calling C from a lot of languages. If it's
broken by design, that's some real news. And it does constructed function calls,
which is exactly what we need here.
On Fri, May 20, 2022 at 12:49:24AM -0400, Kent Overstreet wrote:
> On Thu, May 19, 2022 at 09:06:24PM +0000, David Laight wrote:
...
> > > The goal is to replace most of our %p format extensions with this
> > > interface, and to move pretty-printers out of the core vsprintf.c code -
> >
> > One advantage of the current scheme is that is reasonably safe
> > and easy to use.
> > Perhaps too many extra formats have been added recently.
> > This all seems like a recipe for disaster with functions being
> > called with the wrong number of parameters
> > (I can't think how you can compile-time check it).
>
> We can't check it at compile time yet, it's true - printf format checking will
> need to be extended. But we're already talking about doing that.
I have heard about GCC plugin, which also may check the %p extension usages.
--
With Best Regards,
Andy Shevchenko
On Thu, May 19, 2022 at 09:06:24PM +0000, David Laight wrote:
> I suspect this a very good way to blow the kernel stack.
> The highest stack use is already very likely to be inside
> the printf code in an error path somewhere.
...
> Double copying using a temporary buffer isn't the end of the world.
How can you hold both of these positions simultaneously?