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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
