On Thursday, August 07, 2014 at 06:52:44 PM, Steve Rae wrote:
[...]
I was referring to what you mention below...
852 - Safe printf() functions 853 Define CONFIG_SYS_VSNPRINTF to compile in safe versions of 854 the printf() functions. These are defined in 855 include/vsprintf.h and include snprintf(), vsnprintf() and 856 so on. Code size increase is approximately 300-500 bytes. 857 If this option is not given then these functions will 858 silently discard their buffer size argument - this means 859 you are not getting any overflow checking in this case.
I really don't see the "cautionary statements" here , no . I see that it discards the size checking if this CONFIG_SYS_VSNPRINTF is not enabled, but that does not obstruct the operation of those functions.
I'm really confused: my code ensures that the buffer is not overflowed and that it is terminated properly. If snprintf() (without CONFIG_SYS_VSNPRINTF defined) doesn't provide "any overflow checking", then why would I use it?
That's why I suggested to enable CONFIG_SYS_VSNPRINTF unconditionally. Then your code would not need to duplicate all the overflow checks, would it ?
and the fact that CONFIG_SYS_VSNPRINTF is not defined for armv7 builds, I am
not going to use it....
Is it a problem to define it? Also, even without CONFIG_SYS_VSNPRINTF , the
functions are still available, see the README: 857 If this option is not given then these functions will 858 silently discard their buffer size argument
- this means 859 you are not getting any overflow
checking in this case.
I have yet to see some hard-evidence against using safe printing functions here.
I don't want to be the first to defined it for all of armv7....
Honestly, we should just enable this CONFIG_SYS_VSNPRINTF by default for the good of humanity and all the things, since this unbounded string handling is just evil (see how OpenSSL ended up, partly because of that ... and I am just starting to see the pattern in all the security code). I don't want to go down that road with U-Boot.
So, would you please cook a separate patch to enable this by default, so it would spur the right kind of discussion on this matter ?
I will apologize in advance, but I just don't know anything about SPL or TPL or any other boards (outside of my very limited armv7 and armv8 scope)....
That's OK.
I would be happy to review and test this suggested patch (on our boards), but would be uncomfortable with proposing this patch. Please go ahead and submit a patch, and I'll check it!
The patch would go something like:
#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) #define CONFIG_SYS_VSNPRINTF #endif
and this would go into include/config_cmd_default.h . Unless I'm wrong.