[U-Boot] [PATCH 1/3] vsprintf.c: Always enable CONFIG_SYS_VSNPRINTF

[Peng Fan <van.freenix@gmail.com>]

Hi Tom,

On Thu, Jan 14, 2016 at 01:02:03PM -0500, Tom Rini wrote:
>Enabling this function always removes some class of string saftey issues.
>The size change here in general is about 400 bytes and this seems a reasonable
>trade-off.
>
>Cc: Peng Fan <peng.fan@nxp.com>
>Cc: Peter Robinson <pbrobinson@gmail.com>
>Cc: Fabio Estevam <fabio.estevam@freescale.com>
>Cc: Adrian Alonso <aalonso@freescale.com>
>Cc: Stefano Babic <sbabic@denx.de>
>Cc: Hans de Goede <hdegoede@redhat.com>
>Signed-off-by: Tom Rini <trini@konsulko.com>


Reviewed-and-tested-by: Peng Fan <peng.fan@nxp.com>

Regards,
Peng.
>
>---
>I ran my usual world build of buildman with size comparison options this time
>to test this out.  The only exception to the general size change here are
>q8_a23_tablet_800x480 and gt90h_v4 both in the non-SPL case.  These both grow
>by 3780 bytes in the non-SPL case.  In all cases except those two, when paired
>with the gzwrite patch I posted yesterday we gain all of that back, or more.
>---
> README                              |  9 ---------
> configs/bayleybay_defconfig         |  1 -
> configs/chromebook_link_defconfig   |  1 -
> configs/chromebox_panther_defconfig |  1 -
> configs/coreboot-x86_defconfig      |  1 -
> configs/crownbay_defconfig          |  1 -
> configs/galileo_defconfig           |  1 -
> configs/minnowmax_defconfig         |  1 -
> configs/qemu-x86_defconfig          |  1 -
> configs/sandbox_defconfig           |  1 -
> include/vsprintf.h                  | 12 ------------
> lib/Kconfig                         |  9 ---------
> lib/vsprintf.c                      | 12 ------------
> 13 files changed, 51 deletions(-)
>
>diff --git a/README b/README
>index 5ac2d44..b7ce63d 100644
>--- a/README
>+++ b/README
>@@ -890,15 +890,6 @@ 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_VSNPRINTF 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.
>-		If this option is not given then these functions will
>-		silently discard their buffer size argument - this means
>-		you are not getting any overflow checking in this case.
>-
> - Boot Delay:	CONFIG_BOOTDELAY - in seconds
> 		Delay before automatically booting the default image;
> 		set to -1 to disable autoboot.
>diff --git a/configs/bayleybay_defconfig b/configs/bayleybay_defconfig
>index f462e05..0879d1e 100644
>--- a/configs/bayleybay_defconfig
>+++ b/configs/bayleybay_defconfig
>@@ -37,4 +37,3 @@ CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
>diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig
>index dbfbb97..baa0ed8 100644
>--- a/configs/chromebook_link_defconfig
>+++ b/configs/chromebook_link_defconfig
>@@ -38,5 +38,4 @@ CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
> CONFIG_TPM=y
>diff --git a/configs/chromebox_panther_defconfig b/configs/chromebox_panther_defconfig
>index ed4428f..c368cc0 100644
>--- a/configs/chromebox_panther_defconfig
>+++ b/configs/chromebox_panther_defconfig
>@@ -33,5 +33,4 @@ CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
> CONFIG_TPM=y
>diff --git a/configs/coreboot-x86_defconfig b/configs/coreboot-x86_defconfig
>index cd2be18..fda0db2 100644
>--- a/configs/coreboot-x86_defconfig
>+++ b/configs/coreboot-x86_defconfig
>@@ -25,5 +25,4 @@ CONFIG_TPM_TIS_LPC=y
> CONFIG_USB=y
> CONFIG_DM_USB=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
> CONFIG_TPM=y
>diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig
>index 932d9ec..6bc4b8d 100644
>--- a/configs/crownbay_defconfig
>+++ b/configs/crownbay_defconfig
>@@ -36,4 +36,3 @@ CONFIG_DM_USB=y
> CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
>diff --git a/configs/galileo_defconfig b/configs/galileo_defconfig
>index 0604aa7..925d3ee 100644
>--- a/configs/galileo_defconfig
>+++ b/configs/galileo_defconfig
>@@ -28,4 +28,3 @@ CONFIG_TIMER=y
> CONFIG_USB=y
> CONFIG_DM_USB=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
>diff --git a/configs/minnowmax_defconfig b/configs/minnowmax_defconfig
>index 864fd1b..af6a8ec 100644
>--- a/configs/minnowmax_defconfig
>+++ b/configs/minnowmax_defconfig
>@@ -39,4 +39,3 @@ CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
>diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
>index 8c86931..b0c935c 100644
>--- a/configs/qemu-x86_defconfig
>+++ b/configs/qemu-x86_defconfig
>@@ -30,4 +30,3 @@ CONFIG_VIDEO_VESA=y
> CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> CONFIG_FRAMEBUFFER_VESA_MODE_111=y
> CONFIG_USE_PRIVATE_LIBGCC=y
>-CONFIG_SYS_VSNPRINTF=y
>diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>index 731fc25..caa7336 100644
>--- a/configs/sandbox_defconfig
>+++ b/configs/sandbox_defconfig
>@@ -76,7 +76,6 @@ CONFIG_USB_EMUL=y
> CONFIG_USB_STORAGE=y
> CONFIG_USB_KEYBOARD=y
> CONFIG_SYS_USB_EVENT_POLL=y
>-CONFIG_SYS_VSNPRINTF=y
> CONFIG_CMD_DHRYSTONE=y
> CONFIG_TPM=y
> CONFIG_LZ4=y
>diff --git a/include/vsprintf.h b/include/vsprintf.h
>index b5bc9c1..376f5dd 100644
>--- a/include/vsprintf.h
>+++ b/include/vsprintf.h
>@@ -124,7 +124,6 @@ int sprintf(char *buf, const char *fmt, ...)
> int vsprintf(char *buf, const char *fmt, va_list args);
> char *simple_itoa(ulong i);
> 
>-#ifdef CONFIG_SYS_VSNPRINTF
> /**
>  * Format a string and place it in a buffer
>  *
>@@ -199,17 +198,6 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args);
>  * See the vsprintf() documentation for format string extensions over C99.
>  */
> 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_VSNPRINTF
>- */
>-#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 /* CONFIG_SYS_VSNPRINTF */
> 
> /**
>  * print_grouped_ull() - print a value with digits grouped by ','
>diff --git a/lib/Kconfig b/lib/Kconfig
>index 9d580e4..46d7034 100644
>--- a/lib/Kconfig
>+++ b/lib/Kconfig
>@@ -27,15 +27,6 @@ config SYS_HZ
> 	  get_timer() must operate in milliseconds and this option must be
> 	  set to 1000.
> 
>-config SYS_VSNPRINTF
>-	bool "Enable safe version of sprintf()"
>-	help
>-	  Since sprintf() can overflow its buffer, it is common to use
>-	  snprintf() instead, which knows the buffer size and can avoid
>-	  overflow. However, this does increase code size slightly (for
>-	  Thumb-2, about 420 bytes). Enable this option for safety when
>-	  using sprintf() with data you do not control.
>-
> config USE_TINY_PRINTF
> 	bool "Enable tiny printf() version"
> 	help
>diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>index 24167a1..874a295 100644
>--- a/lib/vsprintf.c
>+++ b/lib/vsprintf.c
>@@ -141,7 +141,6 @@ static noinline char *put_dec(char *buf, uint64_t num)
> #define SMALL	32		/* Must be 32 == 0x20 */
> #define SPECIAL	64		/* 0x */
> 
>-#ifdef CONFIG_SYS_VSNPRINTF
> /*
>  * 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.
>@@ -151,9 +150,6 @@ static noinline char *put_dec(char *buf, uint64_t num)
> 		*(str) = (ch); \
> 	++str; \
> 	} while (0)
>-#else
>-#define ADDCH(str, ch)	(*(str)++ = (ch))
>-#endif
> 
> static char *number(char *buf, char *end, u64 num,
> 		int base, int size, int precision, int type)
>@@ -441,13 +437,11 @@ static int vsnprintf_internal(char *buf, size_t size, const char *fmt,
> 				/* 't' added for ptrdiff_t */
> 	char *end = buf + size;
> 
>-#ifdef CONFIG_SYS_VSNPRINTF
> 	/* Make sure end is always >= buf - do we want this in U-Boot? */
> 	if (end < buf) {
> 		end = ((void *)-1);
> 		size = end - buf;
> 	}
>-#endif
> 	str = buf;
> 
> 	for (; *fmt ; ++fmt) {
>@@ -609,21 +603,16 @@ repeat:
> 			     flags);
> 	}
> 
>-#ifdef CONFIG_SYS_VSNPRINTF
> 	if (size > 0) {
> 		ADDCH(str, '\0');
> 		if (str > end)
> 			end[-1] = '\0';
> 		--str;
> 	}
>-#else
>-	*str = '\0';
>-#endif
> 	/* the trailing null byte doesn't count towards the total */
> 	return str - buf;
> }
> 
>-#ifdef CONFIG_SYS_VSNPRINTF
> int vsnprintf(char *buf, size_t size, const char *fmt,
> 			      va_list args)
> {
>@@ -666,7 +655,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)
>-- 
>2.6.4
>
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot