Le 23/09/2011 22:46, Simon Glass a écrit : > Hi Albert, > > On Fri, Sep 23, 2011 at 1:40 PM, Albert ARIBAUD > <albert.u.b...@aribaud.net> wrote: >> Hi Simon, >> >> Le 23/09/2011 19:38, Simon Glass a écrit : >>> >>> The printf family of functions in U-Boot cannot deal with a situation >>> where >>> the caller provides a buffer which turns out to be too small for the >>> format >>> string. This can result in buffer overflows, stack overflows and other bad >>> behavior. >> >> Indeed overruns can lead to bad behaviors, but in any case, it can never be >> recovered, because at the root, the problem is that the caller provided >> inconsistent arguments to printf. > > Recovery is one thing, but I would settle for just not crashing, which > is the purpose of this patch. We could also easily WARN if that were > considered appropriate here. > >> >> So in essence, you're 'fixing' printf for a design error in printf's caller, >> instead of fixing the design error. > > Well, the nature of a function is that it cannot know what arguments > might be passed to it. It can only assert(), limit check, etc. A limit > check is what this patch aims to add.
I do agree that as a rule, a function should check its arguments, but as any rule, this one has understated assumptions. Here, one assumption is that calls to the functions cannot be made compliant, and therefore it falls to the function to ensure this compliance; and another one is that the function can actually check this conformance. The first assumption is correct in an OS or general-purpose library where the number of callers is virtually unlimited and there is no way to make sure all calls are conforming. In U-Boot however, the number of callers is bound and known (unless you're thinking of standalone U-Boot apps, but I don't see this as a use case so frequent that it warrants a 'fix') The second assumption is false by nature for printf(), which simply cannot check its input buffer (OTOH *nprintf() does, and it is its job). Besides, there *is* a sub-family of printf functions which are dedicated to the case where buffers provided may be too small for the print output -- meaning that the issue is known and an easy fix exists if cropping the output is allowable. So basically the choice is between: - adding code to the printf() family to try and fix an issue that it is fundamentally unable to properly fix, and for which a solution exists, or - grepping and fixing calls to *sprintf() in U-Boot that do not respect the known contraints of printf(), by resizing the buffer or calling *snprintf() instead. I am definitely not in favor of the first option concerning U-Boot. > Regards, > Simon Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
