On Tue, Apr 10, 2018 at 10:42:45AM -0400, Simon Glass wrote: > +U-Boot, Tom, Masahiro > > Hi David, > > On 10 April 2018 at 01:22, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > > 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". I'm either failing in my Google-fu or is there not an easy way to grab the patches from patchwork/similar? But, if you shoot me the series off-list, I can tell you how much U-Boot targets grow here (we can use the same script as the kernel to re-sync sources back in, so I can give you a before/after). But as Simon notes, we have a number of platforms that need to use (parts of) libfdt and stick to ~30KiB or less in total, sometimes including some memory for stack/etc and we've long been using -ffunction-sections/etc (the latest round of trying to use LTO has me thinking maybe we can see if that's a valid option finally, but that's an aside). Thanks! -- Tom
Attachment:
signature.asc
Description: PGP signature
