Buffer overruns in printf
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest0 of 4 patches 0 repliesNew
Patches in v1 No applies recorded yet
None of the 4 patches the cover letter announces are in this archive.
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [U-Boot] [PATCH 0/4] Buffer overruns in printf
Le 23/09/2011 22:46, Simon Glass a ?crit :
Quoted 24 lines
Hi Albert,
Quoted 11 lines · Albert ARIBAUD
Hi Simon,
Le 23/09/2011 19:38, Simon Glass a ?crit :
Quoted 7 lines
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.Quoted 3 lines
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.
Quoted 1 line
Quoted 6 lines
Quoted 2 lines
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.
Quoted 2 lines
Regards,
Simon
Amicalement,
Signature · 1 line
Albert.