On Wed, Apr 04, 2018 at 01:21:10AM +0800, Simon Glass wrote: > Hi David, > > On 3 April 2018 at 23:02, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Fri, Mar 30, 2018 at 04:42:21PM +0800, Simon Glass wrote: > > > Hi David, > > > > > > On 26 March 2018 at 07:25, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > fdt_string() is used to retrieve strings from a DT blob's strings section. > > > > It's rarely used directly, but is widely used internally. > > > > > > > > However, it doesn't do any bounds checking, which means in the case of a > > > > corrupted blob it could access bad memory, which libfdt is supposed to > > > > avoid. > > > > > > > > This write a safe alternative to fdt_string, fdt_get_string(). It checks > > > > both that the given offset is within the string section and that the string > > > > it points to is properly \0 terminated within the section. It also returns > > > > the string's length as a convenience (since it needs to determine to do the > > > > checks anyway). > > > > > > > > fdt_string() is rewritten in terms of fdt_get_string() for compatibility. > > > > > > > > Most of the diff here is actually testing infrastructure. > > > > > > > > Signed-off-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> > > > > --- > > > > libfdt/fdt_ro.c | 61 +++++++++++++++++++++++++++++++++++-- > > > > libfdt/libfdt.h | 18 ++++++++++- > > > > libfdt/version.lds | 2 +- > > > > tests/.gitignore | 1 + > > > > tests/Makefile.tests | 2 +- > > > > tests/run_tests.sh | 1 + > > > > tests/testdata.h | 1 + > > > > tests/testutils.c | 11 +++++-- > > > > tests/trees.S | 26 ++++++++++++++++ > > > > tests/truncated_string.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > > 10 files changed, 193 insertions(+), 9 deletions(-) > > > > create mode 100644 tests/truncated_string.c > > > > > > Similar code-size quesiton here. It looks like a lot of checking code. > > > Can we have an option to remove it? > > > > Again, I'm disinclined without a concrete example of a problem. Fwiw > > the code size change is +276 bytes on my setup. > > That might not sound like a lot, but the overhead of DT in U-Boot is > about 3KB, so this adds nearly 10%. Hm. And how much is it compared to the whole U-Boot blob? > The specific problem is that when U-Boot SPL gets too big boards don't > boot. Because we take the upstream libfdt this will affect U-Boot. > > Do you have any thoughts on how we could avoid this size increase? So, again, I'm very disinclined to prioritize size over memory safety without a *concrete* example. i.e. "We hit this specific problem with size on this specific board that we were really using" rather than just "it might be a problem". IMO, thinking of it in terms of the "increase" is the wrong way arond. If size is really a problem for you, you want to consider how you can reduce it in any way, not just rolling back the most recent changes. The most obvious one to me would be to try -ffunction-sections to exclude any functions that aren't actually used by u-boot (if this is helpful and the compiler's an issue, I'd be willing to consider splitting up libfdt into a bunch more C files). -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
