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

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 sjg@chromium.org) With my ARMv7 compiler (gcc-4.4.3_cos_gg_53174) the code size increase is 328 bytes, about 10% increase to code size vsprintf.o.
The newly added functions (snprintf, vscnprintf, scnprintf) are a total of 116 bytes.
The changes to number(), string() and vsprintf() to make them respect an end pointer increase size by 80, 20 and 80 bytes respectively.
Total text size for existing vsprintf.o functions goes from 0xc10 (3088) to 0xd58 (3416), or 328 bytes. Of this 116 bytes is the new functions and the rest is dealing with the end pointer. There is no data.
With the CONFIG_SYS_VSNPRINT option not defined, the code size impact is 12 bytes, or alternatively zero if the ugly macro patch is applied.
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 - Add ugly macros to reduce code size - Move function documentation into header file
Simon Glass (4): Move vsprintf functions into their own header Add limits.h to hold basic limits vsprintf: Introduce ugly macros to reduce code size 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
README | 6 ++ common/console.c | 10 +- fs/ubifs/ubifs.h | 4 +- include/common.h | 10 +-- include/limits.h | 40 +++++++++ include/vsprintf.h | 180 +++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 241 +++++++++++++++++++++++++++++++++------------------- 7 files changed, 386 insertions(+), 105 deletions(-) create mode 100644 include/limits.h create mode 100644 include/vsprintf.h

common.h is very large, so before changing the vsprintf functions, move the prototypes into their own header file.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Move prototypes from common.h to vsprintf.h
include/common.h | 10 +--------- include/vsprintf.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 include/vsprintf.h
diff --git a/include/common.h b/include/common.h index 4c3e3a6..f24d351 100644 --- a/include/common.h +++ b/include/common.h @@ -708,15 +708,7 @@ void uuid_str_to_bin(const char *uuid, unsigned char *out); int uuid_str_valid(const char *uuid);
/* lib/vsprintf.c */ -ulong simple_strtoul(const char *cp,char **endp,unsigned int base); -int strict_strtoul(const char *cp, unsigned int base, unsigned long *res); -unsigned long long simple_strtoull(const char *cp,char **endp,unsigned int base); -long simple_strtol(const char *cp,char **endp,unsigned int base); -void panic(const char *fmt, ...) - __attribute__ ((format (__printf__, 1, 2), noreturn)); -int sprintf(char * buf, const char *fmt, ...) - __attribute__ ((format (__printf__, 2, 3))); -int vsprintf(char *buf, const char *fmt, va_list args); +#include <vsprintf.h>
/* lib/strmhz.c */ char * strmhz(char *buf, unsigned long hz); diff --git a/include/vsprintf.h b/include/vsprintf.h new file mode 100644 index 0000000..5d57d5f --- /dev/null +++ b/include/vsprintf.h @@ -0,0 +1,38 @@ +/* + * (C) Copyright 2000-2009 + * Wolfgang Denk, DENX Software Engineering, wd@denx.de. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#ifndef __VSPRINTF_H +#define __VSPRINTF_H + +ulong simple_strtoul(const char *cp, char **endp, unsigned int base); +int strict_strtoul(const char *cp, unsigned int base, unsigned long *res); +unsigned long long simple_strtoull(const char *cp, char **endp, + unsigned int base); +long simple_strtol(const char *cp, char **endp, unsigned int base); +void panic(const char *fmt, ...) + __attribute__ ((format (__printf__, 1, 2), noreturn)); +int sprintf(char *buf, const char *fmt, ...) + __attribute__ ((format (__printf__, 2, 3))); +int vsprintf(char *buf, const char *fmt, va_list args); + +#endif

This brings a basic limits.h implementation into U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org --- fs/ubifs/ubifs.h | 4 +--- include/limits.h | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 include/limits.h
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 0af471a..e7b6e43 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -311,9 +311,7 @@ struct file { #define MAX_LFS_FILESIZE 0x7fffffffffffffffUL #endif
-#define INT_MAX ((int)(~0U>>1)) -#define INT_MIN (-INT_MAX - 1) -#define LLONG_MAX ((long long)(~0ULL>>1)) +#include <limits.h>
/* * These are the fs-independent mount-flags: up to 32 flags are supported diff --git a/include/limits.h b/include/limits.h new file mode 100644 index 0000000..1021291 --- /dev/null +++ b/include/limits.h @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2011 The Chromium OS Authors. + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#ifndef __LIMITS_H +#define __LIMITS_H + +/* Keep all our limits in one place */ + +#define USHRT_MAX ((u16)(~0U)) +#define SHRT_MAX ((s16)(USHRT_MAX>>1)) +#define SHRT_MIN ((s16)(-SHRT_MAX - 1)) +#define INT_MAX ((int)(~0U>>1)) +#define INT_MIN (-INT_MAX - 1) +#define UINT_MAX (~0U) +#define LONG_MAX ((long)(~0UL>>1)) +#define LONG_MIN (-LONG_MAX - 1) +#define ULONG_MAX (~0UL) +#define LLONG_MAX ((long long)(~0ULL>>1)) +#define LLONG_MIN (-LLONG_MAX - 1) +#define ULLONG_MAX (~0ULL) + +#endif

From: Sonny Rao sonnyrao@chromium.org
From: Sonny Rao sonnyrao@chromium.org
These functions are useful in U-Boot because they allow a graceful failure rather than an unpredictable stack overflow when printf() buffers are exceeded.
Mostly copied from the Linux kernel. I copied vscnprintf and scnprintf so we can change printf and vprintf to use the safe implementation but still return the correct values.
(Simon Glass sjg@chromium.org modified this commit a little)
Signed-off-by: Sonny Rao sonnyrao@chromium.org --- Changes in v3: - 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
README | 6 + include/vsprintf.h | 19 ++++ lib/vsprintf.c | 262 +++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 235 insertions(+), 52 deletions(-)
diff --git a/README b/README index eb9ade9..f8b90ef 100644 --- a/README +++ b/README @@ -634,6 +634,12 @@ The following options need to be configured: 'Sane' compilers will generate smaller code if CONFIG_PRE_CON_BUF_SZ is a power of 2
+- Safe printf() functions + Define CONFIG_SYS_VSNPRINT to compile in safe versions of + the printf() functions. These are defined in + include/vsprintf.h and include snprintf(), vsnprintf() and + so on. Code size increase is approximately 300-500 bytes. + - Boot Delay: CONFIG_BOOTDELAY - in seconds Delay before automatically booting the default image; set to -1 to disable autoboot. diff --git a/include/vsprintf.h b/include/vsprintf.h index 5d57d5f..adb7483 100644 --- a/include/vsprintf.h +++ b/include/vsprintf.h @@ -35,4 +35,23 @@ int sprintf(char *buf, const char *fmt, ...) __attribute__ ((format (__printf__, 2, 3))); int vsprintf(char *buf, const char *fmt, va_list args);
+#ifdef CONFIG_SYS_VSNPRINT +int snprintf(char *buf, size_t size, const char *fmt, ...) + __attribute__ ((format (__printf__, 3, 4))); +int scnprintf(char *buf, size_t size, const char *fmt, ...) + __attribute__ ((format (__printf__, 3, 4))); +int vsnprintf(char *buf, size_t size, const char *fmt, va_list args); +int vscnprintf(char *buf, size_t size, const char *fmt, va_list args); +#else +/* + * Use macros to silently drop the size parameter. Note that the 'cn' + * versions are the same as the 'n' versions since the functions assume + * there is always enough buffer space when !CONFIG_SYS_VSNPRINT + */ +#define snprintf(buf, size, fmt, args...) sprintf(buf, fmt, ##args) +#define scnprintf(buf, size, fmt, args...) sprintf(buf, fmt, ##args) +#define vsnprintf(buf, size, fmt, args...) vsprintf(buf, fmt, ##args) +#define vscnprintf(buf, size, fmt, args...) vsprintf(buf, fmt, ##args) +#endif + #endif diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 79dead3..5eaf87c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -16,6 +16,7 @@ #include <errno.h>
#include <common.h> +#include <limits.h> #if !defined (CONFIG_PANIC_HANG) #include <command.h> #endif @@ -289,7 +290,22 @@ static noinline char* put_dec(char *buf, unsigned NUM_TYPE num) #define SMALL 32 /* Must be 32 == 0x20 */ #define SPECIAL 64 /* 0x */
-static char *number(char *buf, unsigned NUM_TYPE num, int base, int size, int precision, int type) +#ifdef CONFIG_SYS_VSNPRINT +/* + * Macro to add a new character to our output string, but only if it will + * fit. The macro moves to the next character position in the output string. + */ +#define ADDCH(str, ch) do { \ + if ((str) < end) \ + *(str) = (ch); \ + ++str; \ + } while (0) +#else +#define ADDCH(str, ch) (*(str)++ = (ch)) +#endif + +static char *number(char *buf, char *end, unsigned NUM_TYPE num, + int base, int size, int precision, int type) { /* we are called with base 8, 10 or 16, only, thus don't need "G..." */ static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */ @@ -351,37 +367,40 @@ static char *number(char *buf, unsigned NUM_TYPE num, int base, int size, int pr precision = i; /* leading space padding */ size -= precision; - if (!(type & (ZEROPAD+LEFT))) - while(--size >= 0) - *buf++ = ' '; + if (!(type & (ZEROPAD + LEFT))) { + while (--size >= 0) + ADDCH(buf, ' '); + } /* sign */ if (sign) - *buf++ = sign; + ADDCH(buf, sign); /* "0x" / "0" prefix */ if (need_pfx) { - *buf++ = '0'; + ADDCH(buf, '0'); if (base == 16) - *buf++ = ('X' | locase); + ADDCH(buf, 'X' | locase); } /* zero or space padding */ if (!(type & LEFT)) { char c = (type & ZEROPAD) ? '0' : ' '; + while (--size >= 0) - *buf++ = c; + ADDCH(buf, c); } /* hmm even more zero padding? */ while (i <= --precision) - *buf++ = '0'; + ADDCH(buf, '0'); /* actual digits of result */ while (--i >= 0) - *buf++ = tmp[i]; + ADDCH(buf, tmp[i]); /* trailing space padding */ while (--size >= 0) - *buf++ = ' '; + ADDCH(buf, ' '); return buf; }
-static char *string(char *buf, char *s, int field_width, int precision, int flags) +static char *string(char *buf, char *end, char *s, int field_width, + int precision, int flags) { int len, i;
@@ -392,16 +411,16 @@ static char *string(char *buf, char *s, int field_width, int precision, int flag
if (!(flags & LEFT)) while (len < field_width--) - *buf++ = ' '; + ADDCH(buf, ' '); for (i = 0; i < len; ++i) - *buf++ = *s++; + ADDCH(buf, *s++); while (len < field_width--) - *buf++ = ' '; + ADDCH(buf, ' '); return buf; }
#ifdef CONFIG_CMD_NET -static char *mac_address_string(char *buf, u8 *addr, int field_width, +static char *mac_address_string(char *buf, char *end, u8 *addr, int field_width, int precision, int flags) { char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */ @@ -415,10 +434,11 @@ static char *mac_address_string(char *buf, u8 *addr, int field_width, } *p = '\0';
- return string(buf, mac_addr, field_width, precision, flags & ~SPECIAL); + return string(buf, end, mac_addr, field_width, precision, + flags & ~SPECIAL); }
-static char *ip6_addr_string(char *buf, u8 *addr, int field_width, +static char *ip6_addr_string(char *buf, char *end, u8 *addr, int field_width, int precision, int flags) { char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */ @@ -433,10 +453,11 @@ static char *ip6_addr_string(char *buf, u8 *addr, int field_width, } *p = '\0';
- return string(buf, ip6_addr, field_width, precision, flags & ~SPECIAL); + return string(buf, end, ip6_addr, field_width, precision, + flags & ~SPECIAL); }
-static char *ip4_addr_string(char *buf, u8 *addr, int field_width, +static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width, int precision, int flags) { char ip4_addr[4 * 4]; /* (4 * 3 decimal digits), 3 dots and trailing zero */ @@ -454,7 +475,8 @@ static char *ip4_addr_string(char *buf, u8 *addr, int field_width, } *p = '\0';
- return string(buf, ip4_addr, field_width, precision, flags & ~SPECIAL); + return string(buf, end, ip4_addr, field_width, precision, + flags & ~SPECIAL); } #endif
@@ -476,10 +498,12 @@ static char *ip4_addr_string(char *buf, u8 *addr, int field_width, * function pointers are really function descriptors, which contain a * pointer to the real address. */ -static char *pointer(const char *fmt, char *buf, void *ptr, int field_width, int precision, int flags) +static char *pointer(const char *fmt, char *buf, char *end, void *ptr, + int field_width, int precision, int flags) { if (!ptr) - return string(buf, "(null)", field_width, precision, flags); + return string(buf, end, "(null)", field_width, precision, + flags);
#ifdef CONFIG_CMD_NET switch (*fmt) { @@ -487,15 +511,18 @@ static char *pointer(const char *fmt, char *buf, void *ptr, int field_width, int flags |= SPECIAL; /* Fallthrough */ case 'M': - return mac_address_string(buf, ptr, field_width, precision, flags); + return mac_address_string(buf, end, ptr, field_width, + precision, flags); case 'i': flags |= SPECIAL; /* Fallthrough */ case 'I': if (fmt[1] == '6') - return ip6_addr_string(buf, ptr, field_width, precision, flags); + return ip6_addr_string(buf, end, ptr, field_width, + precision, flags); if (fmt[1] == '4') - return ip4_addr_string(buf, ptr, field_width, precision, flags); + return ip4_addr_string(buf, end, ptr, field_width, + precision, flags); flags &= ~SPECIAL; break; } @@ -505,27 +532,31 @@ static char *pointer(const char *fmt, char *buf, void *ptr, int field_width, int field_width = 2*sizeof(void *); flags |= ZEROPAD; } - return number(buf, (unsigned long) ptr, 16, field_width, precision, flags); + return number(buf, end, (unsigned long)ptr, 16, field_width, + precision, flags); }
/** - * vsprintf - Format a string and place it in a buffer - * @buf: The buffer to place the result into - * @fmt: The format string to use - * @args: Arguments for the format string + * Format a string and place it in a buffer (base function) * - * This function follows C99 vsprintf, but has some extensions: + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param args Arguments for the format string + * @return The number characters which would be generated for the given + * input, excluding the trailing '\0', as per ISO C99. Note that fewer + * characters may be written if this number of characters is >= size. + * + * This function follows C99 vsnprintf, but has some extensions: * %pS output the name of a text symbol * %pF output the name of a function pointer * %pR output the address range in a struct resource * - * The function returns the number of characters written - * into @buf. - * * Call this function if you are already dealing with a va_list. - * You probably want sprintf() instead. + * You probably want snprintf() instead. */ -int vsprintf(char *buf, const char *fmt, va_list args) +static int vsnprintf_internal(char *buf, size_t size, const char *fmt, + va_list args) { unsigned NUM_TYPE num; int base; @@ -540,12 +571,19 @@ int vsprintf(char *buf, const char *fmt, va_list args) /* 'z' support added 23/7/1999 S.H. */ /* 'z' changed to 'Z' --davidm 1/25/99 */ /* 't' added for ptrdiff_t */ + char *end = buf + size; + + /* Make sure end is always >= buf - do we want this in U-Boot? */ + if (end < buf) { + end = ((void *)-1); + size = end - buf; + }
str = buf;
for (; *fmt ; ++fmt) { if (*fmt != '%') { - *str++ = *fmt; + ADDCH(str, *fmt); continue; }
@@ -607,20 +645,22 @@ int vsprintf(char *buf, const char *fmt, va_list args)
switch (*fmt) { case 'c': - if (!(flags & LEFT)) + if (!(flags & LEFT)) { while (--field_width > 0) - *str++ = ' '; - *str++ = (unsigned char) va_arg(args, int); + ADDCH(str, ' '); + } + ADDCH(str, (unsigned char) va_arg(args, int)); while (--field_width > 0) - *str++ = ' '; + ADDCH(str, ' '); continue;
case 's': - str = string(str, va_arg(args, char *), field_width, precision, flags); + str = string(str, end, va_arg(args, char *), + field_width, precision, flags); continue;
case 'p': - str = pointer(fmt+1, str, + str = pointer(fmt+1, str, end, va_arg(args, void *), field_width, precision, flags); /* Skip all alphanumeric pointer suffixes */ @@ -639,7 +679,7 @@ int vsprintf(char *buf, const char *fmt, va_list args) continue;
case '%': - *str++ = '%'; + ADDCH(str, '%'); continue;
/* integer number formats - set up the flags and "break" */ @@ -660,9 +700,9 @@ int vsprintf(char *buf, const char *fmt, va_list args) break;
default: - *str++ = '%'; + ADDCH(str, '%'); if (*fmt) - *str++ = *fmt; + ADDCH(str, *fmt); else --fmt; continue; @@ -686,17 +726,135 @@ int vsprintf(char *buf, const char *fmt, va_list args) if (flags & SIGN) num = (signed int) num; } - str = number(str, num, base, field_width, precision, flags); + str = number(str, end, num, base, field_width, precision, + flags); + } + +#ifdef CONFIG_SYS_VSNPRINT + if (size > 0) { + ADDCH(str, '\0'); + if (str > end) + end[-1] = '\0'; } +#else *str = '\0'; +#endif + /* the trailing null byte doesn't count towards the total */ return str-buf; }
+#ifdef CONFIG_SYS_VSNPRINT +int vsnprintf(char *buf, size_t size, const char *fmt, + va_list args) +{ + return vsnprintf_internal(buf, size, fmt, args); +} + +/** + * Format a string and place it in a buffer (va_list version) + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param args Arguments for the format string + * @return the number of characters which have been written into + * the @buf not including the trailing '\0'. If @size is == 0 the function + * returns 0. + * + * If you're not already dealing with a va_list consider using scnprintf(). + * + * See the vsprintf() documentation for format string extensions over C99. + */ +int vscnprintf(char *buf, size_t size, const char *fmt, va_list args) +{ + int i; + + i = vsnprintf(buf, size, fmt, args); + + if (likely(i < size)) + return i; + if (size != 0) + return size - 1; + return 0; +} + +/** + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param ... Arguments for the format string + * @return the number of characters which would be + * generated for the given input, excluding the trailing null, + * as per ISO C99. If the return is greater than or equal to + * @size, the resulting string is truncated. + * + * See the vsprintf() documentation for format string extensions over C99. + */ +int snprintf(char *buf, size_t size, const char *fmt, ...) +{ + va_list args; + int i; + + va_start(args, fmt); + i = vsnprintf(buf, size, fmt, args); + va_end(args); + + return i; +} + /** - * sprintf - Format a string and place it in a buffer - * @buf: The buffer to place the result into - * @fmt: The format string to use - * @...: Arguments for the format string + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param ... Arguments for the format string + * + * The return value is the number of characters written into @buf not including + * the trailing '\0'. If @size is == 0 the function returns 0. + * + * See the vsprintf() documentation for format string extensions over C99. + */ + +int scnprintf(char *buf, size_t size, const char *fmt, ...) +{ + va_list args; + int i; + + va_start(args, fmt); + i = vscnprintf(buf, size, fmt, args); + va_end(args); + + return i; +} +#endif /* CONFIG_SYS_VSNPRINT */ + +/** + * Format a string and place it in a buffer (va_list version) + * + * @param buf The buffer to place the result into + * @param fmt The format string to use + * @param args Arguments for the format string + * + * The function returns the number of characters written + * into @buf. Use vsnprintf() or vscnprintf() in order to avoid + * buffer overflows. + * + * If you're not already dealing with a va_list consider using sprintf(). + */ +int vsprintf(char *buf, const char *fmt, va_list args) +{ + return vsnprintf_internal(buf, INT_MAX, fmt, args); +} + +/** + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param fmt The format string to use + * @param ... Arguments for the format string * * The function returns the number of characters written * into @buf.

Even with CONFIG_SYS_VSNPRINT not defined, the code size is a little larger (12 bytes on ARM). This ugly change could be used to fix that. I am very doubtful that we want to go this far.
Even if we do, perhaps we would be better defining the functions as normal (with an end parameter) and then using macros to optionally remove that parameter. But neither option is great so I am sending this as is for comment. Hopefully there is a better way, or we can just drop this patch.
This generates a checkpatch warning: error: lib/vsprintf.c,309: Macros with complex values should be enclosed in parenthesis
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Add ugly macros to reduce code size
lib/vsprintf.c | 59 +++++++++++++++++++++++++++++++------------------------ 1 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 5eaf87c..65c0984 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -300,11 +300,20 @@ static noinline char* put_dec(char *buf, unsigned NUM_TYPE num) *(str) = (ch); \ ++str; \ } while (0) + +/* + * Macros to declare and use the end pointer, depending on whether we are + * using one or not. + */ +#define DECLARE_END char *end, +#define USE_END end, #else #define ADDCH(str, ch) (*(str)++ = (ch)) +#define DECLARE_END +#define USE_END #endif
-static char *number(char *buf, char *end, unsigned NUM_TYPE num, +static char *number(char *buf, DECLARE_END unsigned NUM_TYPE num, int base, int size, int precision, int type) { /* we are called with base 8, 10 or 16, only, thus don't need "G..." */ @@ -399,7 +408,7 @@ static char *number(char *buf, char *end, unsigned NUM_TYPE num, return buf; }
-static char *string(char *buf, char *end, char *s, int field_width, +static char *string(char *buf, DECLARE_END char *s, int field_width, int precision, int flags) { int len, i; @@ -420,8 +429,8 @@ static char *string(char *buf, char *end, char *s, int field_width, }
#ifdef CONFIG_CMD_NET -static char *mac_address_string(char *buf, char *end, u8 *addr, int field_width, - int precision, int flags) +static char *mac_address_string(char *buf, DECLARE_END u8 *addr, + int field_width, int precision, int flags) { char mac_addr[6 * 3]; /* (6 * 2 hex digits), 5 colons and trailing zero */ char *p = mac_addr; @@ -438,7 +447,7 @@ static char *mac_address_string(char *buf, char *end, u8 *addr, int field_width, flags & ~SPECIAL); }
-static char *ip6_addr_string(char *buf, char *end, u8 *addr, int field_width, +static char *ip6_addr_string(char *buf, DECLARE_END u8 *addr, int field_width, int precision, int flags) { char ip6_addr[8 * 5]; /* (8 * 4 hex digits), 7 colons and trailing zero */ @@ -457,7 +466,7 @@ static char *ip6_addr_string(char *buf, char *end, u8 *addr, int field_width, flags & ~SPECIAL); }
-static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width, +static char *ip4_addr_string(char *buf, DECLARE_END u8 *addr, int field_width, int precision, int flags) { char ip4_addr[4 * 4]; /* (4 * 3 decimal digits), 3 dots and trailing zero */ @@ -498,11 +507,11 @@ static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width, * function pointers are really function descriptors, which contain a * pointer to the real address. */ -static char *pointer(const char *fmt, char *buf, char *end, void *ptr, +static char *pointer(const char *fmt, char *buf, DECLARE_END void *ptr, int field_width, int precision, int flags) { if (!ptr) - return string(buf, end, "(null)", field_width, precision, + return string(buf, USE_END "(null)", field_width, precision, flags);
#ifdef CONFIG_CMD_NET @@ -511,17 +520,17 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, flags |= SPECIAL; /* Fallthrough */ case 'M': - return mac_address_string(buf, end, ptr, field_width, + return mac_address_string(buf, USE_END ptr, field_width, precision, flags); case 'i': flags |= SPECIAL; /* Fallthrough */ case 'I': if (fmt[1] == '6') - return ip6_addr_string(buf, end, ptr, field_width, + return ip6_addr_string(buf, USE_END ptr, field_width, precision, flags); if (fmt[1] == '4') - return ip4_addr_string(buf, end, ptr, field_width, + return ip4_addr_string(buf, USE_END ptr, field_width, precision, flags); flags &= ~SPECIAL; break; @@ -532,7 +541,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, field_width = 2*sizeof(void *); flags |= ZEROPAD; } - return number(buf, end, (unsigned long)ptr, 16, field_width, + return number(buf, USE_END(unsigned long)ptr, 16, field_width, precision, flags); }
@@ -555,8 +564,11 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, * Call this function if you are already dealing with a va_list. * You probably want snprintf() instead. */ -static int vsnprintf_internal(char *buf, size_t size, const char *fmt, - va_list args) +#ifdef CONFIG_SYS_VSNPRINT +int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) +#else +int vsprintf(char *buf, const char *fmt, va_list args) +#endif { unsigned NUM_TYPE num; int base; @@ -571,6 +583,7 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt, /* 'z' support added 23/7/1999 S.H. */ /* 'z' changed to 'Z' --davidm 1/25/99 */ /* 't' added for ptrdiff_t */ +#ifdef CONFIG_SYS_VSNPRINT char *end = buf + size;
/* Make sure end is always >= buf - do we want this in U-Boot? */ @@ -578,7 +591,7 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt, end = ((void *)-1); size = end - buf; } - +#endif str = buf;
for (; *fmt ; ++fmt) { @@ -655,12 +668,12 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt, continue;
case 's': - str = string(str, end, va_arg(args, char *), + str = string(str, USE_END va_arg(args, char *), field_width, precision, flags); continue;
case 'p': - str = pointer(fmt+1, str, end, + str = pointer(fmt+1, str, USE_END va_arg(args, void *), field_width, precision, flags); /* Skip all alphanumeric pointer suffixes */ @@ -726,7 +739,7 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt, if (flags & SIGN) num = (signed int) num; } - str = number(str, end, num, base, field_width, precision, + str = number(str, USE_END num, base, field_width, precision, flags); }
@@ -744,12 +757,6 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt, }
#ifdef CONFIG_SYS_VSNPRINT -int vsnprintf(char *buf, size_t size, const char *fmt, - va_list args) -{ - return vsnprintf_internal(buf, size, fmt, args); -} - /** * Format a string and place it in a buffer (va_list version) * @@ -829,7 +836,6 @@ int scnprintf(char *buf, size_t size, const char *fmt, ...)
return i; } -#endif /* CONFIG_SYS_VSNPRINT */
/** * Format a string and place it in a buffer (va_list version) @@ -846,8 +852,9 @@ int scnprintf(char *buf, size_t size, const char *fmt, ...) */ int vsprintf(char *buf, const char *fmt, va_list args) { - return vsnprintf_internal(buf, INT_MAX, fmt, args); + return vsnprintf(buf, INT_MAX, fmt, args); } +#endif /* CONFIG_SYS_VSNPRINT */
/** * Format a string and place it in a buffer

Now that this is not in common.h, perhaps it is acceptable to move this documentation into the header file.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Move function documentation into header file
include/vsprintf.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 100 ------------------------------------------ 2 files changed, 123 insertions(+), 100 deletions(-)
diff --git a/include/vsprintf.h b/include/vsprintf.h index adb7483..31a6fc0 100644 --- a/include/vsprintf.h +++ b/include/vsprintf.h @@ -25,22 +25,145 @@ #define __VSPRINTF_H
ulong simple_strtoul(const char *cp, char **endp, unsigned int base); + +/** + * strict_strtoul - convert a string to an unsigned long strictly + * @param cp The string to be converted + * @param base The number base to use + * @param res The converted result value + * @return 0 if conversion is successful and *res is set to the converted + * value, otherwise it returns -EINVAL and *res is set to 0. + * + * strict_strtoul converts a string to an unsigned long only if the + * string is really an unsigned long string, any string containing + * any invalid char at the tail will be rejected and -EINVAL is returned, + * only a newline char at the tail is acceptible because people generally + * change a module parameter in the following way: + * + * echo 1024 > /sys/module/e1000/parameters/copybreak + * + * echo will append a newline to the tail. + * + * simple_strtoul just ignores the successive invalid characters and + * return the converted value of prefix part of the string. + * + * Copied this function from Linux 2.6.38 commit ID: + * 521cb40b0c44418a4fd36dc633f575813d59a43d + * + */ int strict_strtoul(const char *cp, unsigned int base, unsigned long *res); unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base); long simple_strtol(const char *cp, char **endp, unsigned int base); void panic(const char *fmt, ...) __attribute__ ((format (__printf__, 1, 2), noreturn)); + +/** + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param fmt The format string to use + * @param ... Arguments for the format string + * + * The function returns the number of characters written + * into @buf. + * + * See the vsprintf() documentation for format string extensions over C99. + */ int sprintf(char *buf, const char *fmt, ...) __attribute__ ((format (__printf__, 2, 3))); + +/** + * Format a string and place it in a buffer (va_list version) + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param args Arguments for the format string + * @return the number of characters which have been written into + * the @buf not including the trailing '\0'. If @size is == 0 the function + * returns 0. + * + * If you're not already dealing with a va_list consider using scnprintf(). + * + * See the vsprintf() documentation for format string extensions over C99. + */ int vsprintf(char *buf, const char *fmt, va_list args);
#ifdef CONFIG_SYS_VSNPRINT +/** + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param ... Arguments for the format string + * @return the number of characters which would be + * generated for the given input, excluding the trailing null, + * as per ISO C99. If the return is greater than or equal to + * @size, the resulting string is truncated. + * + * See the vsprintf() documentation for format string extensions over C99. + */ int snprintf(char *buf, size_t size, const char *fmt, ...) __attribute__ ((format (__printf__, 3, 4))); + +/** + * Format a string and place it in a buffer + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param ... Arguments for the format string + * + * The return value is the number of characters written into @buf not including + * the trailing '\0'. If @size is == 0 the function returns 0. + * + * See the vsprintf() documentation for format string extensions over C99. + */ int scnprintf(char *buf, size_t size, const char *fmt, ...) __attribute__ ((format (__printf__, 3, 4))); + +/** + * Format a string and place it in a buffer (base function) + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param args Arguments for the format string + * @return The number characters which would be generated for the given + * input, excluding the trailing '\0', as per ISO C99. Note that fewer + * characters may be written if this number of characters is >= size. + * + * This function follows C99 vsnprintf, but has some extensions: + * %pS output the name of a text symbol + * %pF output the name of a function pointer + * %pR output the address range in a struct resource + * + * The function returns the number of characters which would be + * generated for the given input, excluding the trailing '\0', + * as per ISO C99. + * + * Call this function if you are already dealing with a va_list. + * You probably want snprintf() instead. + */ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args); + +/** + * Format a string and place it in a buffer (va_list version) + * + * @param buf The buffer to place the result into + * @param size The size of the buffer, including the trailing null space + * @param fmt The format string to use + * @param args Arguments for the format string + * @return the number of characters which have been written into + * the @buf not including the trailing '\0'. If @size is == 0 the function + * returns 0. + * + * If you're not already dealing with a va_list consider using scnprintf(). + * + * See the vsprintf() documentation for format string extensions over C99. + */ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args); #else /* diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 65c0984..707a46c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -63,32 +63,6 @@ unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base) return result; }
-/** - * strict_strtoul - convert a string to an unsigned long strictly - * @cp: The string to be converted - * @base: The number base to use - * @res: The converted result value - * - * strict_strtoul converts a string to an unsigned long only if the - * string is really an unsigned long string, any string containing - * any invalid char at the tail will be rejected and -EINVAL is returned, - * only a newline char at the tail is acceptible because people generally - * change a module parameter in the following way: - * - * echo 1024 > /sys/module/e1000/parameters/copybreak - * - * echo will append a newline to the tail. - * - * It returns 0 if conversion is successful and *res is set to the converted - * value, otherwise it returns -EINVAL and *res is set to 0. - * - * simple_strtoul just ignores the successive invalid characters and - * return the converted value of prefix part of the string. - * - * Copied this function from Linux 2.6.38 commit ID: - * 521cb40b0c44418a4fd36dc633f575813d59a43d - * - */ int strict_strtoul(const char *cp, unsigned int base, unsigned long *res) { char *tail; @@ -545,25 +519,6 @@ static char *pointer(const char *fmt, char *buf, DECLARE_END void *ptr, precision, flags); }
-/** - * Format a string and place it in a buffer (base function) - * - * @param buf The buffer to place the result into - * @param size The size of the buffer, including the trailing null space - * @param fmt The format string to use - * @param args Arguments for the format string - * @return The number characters which would be generated for the given - * input, excluding the trailing '\0', as per ISO C99. Note that fewer - * characters may be written if this number of characters is >= size. - * - * This function follows C99 vsnprintf, but has some extensions: - * %pS output the name of a text symbol - * %pF output the name of a function pointer - * %pR output the address range in a struct resource - * - * Call this function if you are already dealing with a va_list. - * You probably want snprintf() instead. - */ #ifdef CONFIG_SYS_VSNPRINT int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) #else @@ -757,21 +712,6 @@ int vsprintf(char *buf, const char *fmt, va_list args) }
#ifdef CONFIG_SYS_VSNPRINT -/** - * Format a string and place it in a buffer (va_list version) - * - * @param buf The buffer to place the result into - * @param size The size of the buffer, including the trailing null space - * @param fmt The format string to use - * @param args Arguments for the format string - * @return the number of characters which have been written into - * the @buf not including the trailing '\0'. If @size is == 0 the function - * returns 0. - * - * If you're not already dealing with a va_list consider using scnprintf(). - * - * See the vsprintf() documentation for format string extensions over C99. - */ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args) { int i; @@ -785,20 +725,6 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args) return 0; }
-/** - * Format a string and place it in a buffer - * - * @param buf The buffer to place the result into - * @param size The size of the buffer, including the trailing null space - * @param fmt The format string to use - * @param ... Arguments for the format string - * @return the number of characters which would be - * generated for the given input, excluding the trailing null, - * as per ISO C99. If the return is greater than or equal to - * @size, the resulting string is truncated. - * - * See the vsprintf() documentation for format string extensions over C99. - */ int snprintf(char *buf, size_t size, const char *fmt, ...) { va_list args; @@ -811,20 +737,6 @@ int snprintf(char *buf, size_t size, const char *fmt, ...) return i; }
-/** - * Format a string and place it in a buffer - * - * @param buf The buffer to place the result into - * @param size The size of the buffer, including the trailing null space - * @param fmt The format string to use - * @param ... Arguments for the format string - * - * The return value is the number of characters written into @buf not including - * the trailing '\0'. If @size is == 0 the function returns 0. - * - * See the vsprintf() documentation for format string extensions over C99. - */ - int scnprintf(char *buf, size_t size, const char *fmt, ...) { va_list args; @@ -856,18 +768,6 @@ int vsprintf(char *buf, const char *fmt, va_list args) } #endif /* CONFIG_SYS_VSNPRINT */
-/** - * Format a string and place it in a buffer - * - * @param buf The buffer to place the result into - * @param fmt The format string to use - * @param ... Arguments for the format string - * - * The function returns the number of characters written - * into @buf. - * - * See the vsprintf() documentation for format string extensions over C99. - */ int sprintf(char * buf, const char *fmt, ...) { va_list args;

From: Sonny Rao sonnyrao@chromium.org
From: Sonny Rao sonnyrao@chromium.org
utilize the added vscnprintf functions to avoid buffer overruns The implementation is fairly dumb in that it doesn't detect that the buffer is too small, but at least will not cause crashes.
Signed-off-by: Sonny Rao sonnyrao@chromium.org --- Changes in v2: - Use sizeof(printbuffer) instead of CONFIG_SYS_PBSIZE - Drop patch which changes network code to use snprintf()
common/console.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/console.c b/common/console.c index f17875e..1177f7d 100644 --- a/common/console.c +++ b/common/console.c @@ -212,7 +212,7 @@ int serial_printf(const char *fmt, ...) /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args); va_end(args);
serial_puts(printbuffer); @@ -281,7 +281,7 @@ int fprintf(int file, const char *fmt, ...) /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args); va_end(args);
/* Send to desired file */ @@ -426,7 +426,7 @@ int printf(const char *fmt, ...) /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args); va_end(args);
/* Print the string */ @@ -447,7 +447,7 @@ int vprintf(const char *fmt, va_list args) /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);
/* Print the string */ puts(printbuffer); @@ -514,7 +514,7 @@ inline void dbg(const char *fmt, ...) /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i = vsnprintf(printbuffer, sizeof(printbuffer), fmt, args); va_end(args);
if ((screen + sizeof(screen) - 1 - cursor)
participants (1)
-
Simon Glass