Hi, On Tue, Oct 25, 2011 at 4:42 PM, Simon Glass <s...@chromium.org> wrote: > 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. > > This patch series tidies this up in the common vsprintf.c code. > > You can find a discussion of the Linux / U-Boot licensing issues here: > > http://patchwork.ozlabs.org/patch/116161/ > > Code Size Impact > ---------------- > > (From Simon Glass <s...@chromium.org>) > With my ARMv7 compiler (gcc-4.4.3_cos_gg_53174) the code size increase is > 312 bytes, about 10% increase to code size vsprintf.o. > > With the CONFIG_SYS_NO_VSNPRINT option defined, the code size impact > is 4 bytes. > > > Changes in v2: > - Use sizeof(printbuffer) instead of CONFIG_SYS_PBSIZE > - Drop patch which changes network code to use snprintf() > > Changes in v3: > - Move prototypes from common.h to vsprintf.h > - Add CONFIG_SYS_VSNPRINT option to enable vsnprintf() functions > - Update README with CONFIG_SYS_VSNPRINT docs > - Use ADDCH macro to support checking/not checking end pointer > - Move function documentation into header file > > Changes in v4: > - Add these changes in unless CONFIG_NO_SYS_VSNPRINT is defined > - Reduce code size overhead if disabled to only 4 bytes on ARM > - Remove the ugly #ifdef patch from series since it only saves 4 bytes > > Changes in v5: > - Define INT_MAX locally within vsprintf.c > - Drop limits.h as it is used in only two places in U-Boot > > Simon Glass (2): > Move vsprintf functions into their own header > vsprintf: Move function documentation into header file > > Sonny Rao (2): > Add safe vsnprintf and snprintf library functions > Make printf and vprintf safe from buffer overruns >
Any more comments on this? I have suggested making these functions available by default, and having CONFIG_NO_SYS_VSNPRINT to remove them. The rationale is that it if CONFIG_NO_SYS_VSNPRINT is not defined, then snprintf() will not check the buffer length (all the code to do it is removed), and this is unexpected. In this case snprintf() is #defined to sprintf(). There is therefore a code size increase by default with this series. We can instead use CONFIG_SYS_VSNPRINT if people think it is safe. Regards, Simon > README | 7 ++ > common/console.c | 10 +- > include/common.h | 10 +-- > include/vsprintf.h | 180 +++++++++++++++++++++++++++++++++++++++ > lib/vsprintf.c | 237 ++++++++++++++++++++++++++++++++------------------- > 5 files changed, 342 insertions(+), 102 deletions(-) > create mode 100644 include/vsprintf.h > > -- > 1.7.3.1 > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
