[U-Boot] [PATCH 0/4] Buffer overruns in printf

[Scott Wood]

On 09/26/2011 06:20 AM, Albert ARIBAUD wrote:
> Hi Simon,
> 
> Le 25/09/2011 16:50, Simon Glass a écrit :
> 
>>> Basically, printf family functions which do not have the 'n' are *know* by
>>> all -- experienced enough :) -- programmers to be *unsafe* (but to require
>>> less from the caller)

printf()/fprintf() are usually safe enough for the things they're used for.

> and it should remain so: no programmer should ever
>>> encounter an implementation of printf that pretends to be even somewhat
>>> safe, because it might bite him/her elsewhere, in another project based on
>>> another C library where printf is just the beartrap it usually is.

And we should mount large spikes on every steering wheel to make people
drive more carefully. :-P

> I should clarify indeed. My opinion, expressed as a single general rule, 
> is "keep the known semantics of *printf() function family unchanged in 
> U-Boot wrt all other printif implementations around". Thus I think that:

FWIW, Linux's printk() truncates output if it exceeds the internal
buffer.  glibc's is an ugly mess, but appears to not need a fixed printf
buffer -- it streams output into its file buffering mechanism.  That
there may be some crappy implementations out there is no excuse for
making these functions difficult to use everywhere.

> Conversively, implementers of the printf() function need not 
> consider any specific recovery action with regard to size issues. For 
> instance, why do we have an internal buffer for printf to begin with? 
> The implementation does not need them 

Apparently this implementation does.  I suppose you could rewrite the
code to not need one, but is that really worthwhile here, compared to
this easy and virtually cost-free (once you have snprintf) form of
damage control?  It's just passing in a meaningful number to vsnprintf
instead of INT_MAX.  You could save some bytes by removing vsprintf(),
as well.

> (and besides, I guess the buffer 
> does not respect the single C99 environmental constraint for printf, 
> that any field should be able to be at least 4095 bytes -- no kidding).

That this minimum is more than most boards allow for the entire output,
and that it is board-specific (so it could work for one board but fail
for another), are all the more reason to want some safety here.  I'd
much rather see a truncated message (and proceed to fix the obvious bug)
than have to debug corruption and possibly recover a bricked board.

> - users who actually wisht to limit outpout ca use either

You say "actually wish to limit output" as if "let it corrupt memory if
it's too large" is the normal thing to want.

-scott