On Fri, Jun 08, 2018 at 04:10:04PM +1000, Alexey Kardashevskiy wrote: > On 7/6/18 7:37 pm, David Gibson wrote: > > On Thu, Jun 07, 2018 at 04:21:48PM +1000, Alexey Kardashevskiy wrote: > >> On 4/6/18 11:13 pm, David Gibson wrote: > >>> On Sun, May 27, 2018 at 08:24:53PM -0600, Simon Glass wrote: > >>>> Hi David, > >>>> > >>>> On 12 April 2018 at 16:23, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > >>>>> > >>>>> On Thu, Apr 12, 2018 at 11:28:49AM -0600, Simon Glass wrote: > >>>>>> Hi David, > >>>>>> > >>>>>> On 11 April 2018 at 22:52, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > >>>>>>> It's always been a design goal of libfdt that it should be safe to > >>>>>>> use, even on a badly corrupted fdt image. "Safe" here meaning that it > >>>>>>> should never access memory outside the blob's stated boundaries. > >>>>>>> > >>>>>>> The existing code is pretty good about this with regards to accesses > >>>>>>> to the structure block, thanks to the fdt_offset_ptr() helper > >>>>>>> function. However, accesses to the strings and memory reservation > >>>>>>> blocks were less careful and could easily lead to wild pointer > >>>>>>> dereferences in the case of a bad blob. > >>>>>>> > >>>>>>> This series makes a number of improvements to libfdt safety, capping > >>>>>>> it off with a new fdt_check_full() function which acts like an "fsck" > >>>>>>> for fdt blobs. Along the way we make some other cleanups. > >>>>>>> > >>>>>>> Changes since v1: > >>>>>>> * Assorted minor polishes based on review comments > >>>>>>> * Added an extra patch to clean up sequential write mode state > >>>>>>> checking > >>>>>>> > >>>>>>> David Gibson (13): > >>>>>>> libfdt: Clean up header checking functions > >>>>>>> libfdt: Improve sequential write state checking > >>>>>>> libfdt: Make fdt_check_header() more thorough > >>>>>>> libfdt: Safer access to strings section > >>>>>>> libfdt: Propagate name errors in fdt_getprop_by_offset() > >>>>>>> libfdt: Safer access to memory reservations > >>>>>>> Consolidate utilfdt_read_len() variants > >>>>>>> libfdt: Add fdt_header_size() > >>>>>>> Use size_t for blob lengths in utilfdt_read* > >>>>>>> tests: Remove unused #define > >>>>>>> tests: Better handling of valgrind errors saving blobs > >>>>>>> tests: Use valgrind client requests for better checking > >>>>>>> libfdt: Add fdt_check_full() function > >>>>>>> > >>>>>>> fdtdump.c | 4 +- > >>>>>>> fdtget.c | 2 +- > >>>>>>> fdtoverlay.c | 6 +- > >>>>>>> fdtput.c | 2 +- > >>>>>>> libfdt/fdt.c | 76 +++++++++++++++++- > >>>>>>> libfdt/fdt_overlay.c | 6 +- > >>>>>>> libfdt/fdt_ro.c | 189 ++++++++++++++++++++++++++++++++++++++++----- > >>>>>>> libfdt/fdt_rw.c | 28 +++---- > >>>>>>> libfdt/fdt_sw.c | 97 ++++++++++++++++++----- > >>>>>>> libfdt/libfdt.h | 35 ++++++++- > >>>>>>> libfdt/libfdt_env.h | 1 + > >>>>>>> libfdt/libfdt_internal.h | 5 +- > >>>>>>> libfdt/version.lds | 2 +- > >>>>>>> tests/.gitignore | 5 ++ > >>>>>>> tests/Makefile.tests | 6 +- > >>>>>>> tests/check_full.c | 63 +++++++++++++++ > >>>>>>> tests/check_header.c | 128 ++++++++++++++++++++++++++++++ > >>>>>>> tests/dumptrees.c | 2 + > >>>>>>> tests/mangle-layout.c | 8 +- > >>>>>>> tests/mangle-layout.supp | 7 -- > >>>>>>> tests/open_pack.supp | 7 -- > >>>>>>> tests/run_tests.sh | 14 ++++ > >>>>>>> tests/sw_states.c | 140 +++++++++++++++++++++++++++++++++ > >>>>>>> tests/sw_tree1.supp | 18 ----- > >>>>>>> tests/testdata.h | 2 + > >>>>>>> tests/tests.h | 1 + > >>>>>>> tests/testutils.c | 78 +++++++++++++++++-- > >>>>>>> tests/trees.S | 46 +++++++++++ > >>>>>>> tests/truncated_memrsv.c | 63 +++++++++++++++ > >>>>>>> tests/truncated_property.c | 2 + > >>>>>>> tests/truncated_string.c | 81 +++++++++++++++++++ > >>>>>>> util.c | 23 ++---- > >>>>>>> util.h | 20 +---- > >>>>>>> 33 files changed, 1016 insertions(+), 151 deletions(-) > >>>>>>> create mode 100644 tests/check_full.c > >>>>>>> create mode 100644 tests/check_header.c > >>>>>>> delete mode 100644 tests/mangle-layout.supp > >>>>>>> delete mode 100644 tests/open_pack.supp > >>>>>>> create mode 100644 tests/sw_states.c > >>>>>>> delete mode 100644 tests/sw_tree1.supp > >>>>>>> create mode 100644 tests/truncated_memrsv.c > >>>>>>> create mode 100644 tests/truncated_string.c > >>>>>> > >>>>>> Is there a patchwork client I can use to try these patches please, or > >>>>>> a git tree somewhere? For some reason I cannot find it. > >>>>> > >>>>> There's no patchwork, but you can get the git tree at > >>>>> https://github.com/dgibson/dtc/tree/safety > >>>> > >>>> OK thank you. > >>>> > >>>> It is probably best that you apply this and I will see if I can take a > >>>> look at cutting the size back afterwards. > >>> > >>> Ok, will do. > >>> > >>>> How about something like: > >>>> > >>>> static inline fdt_do_checks(void) > >>>> { > >>>> #ifdef FDT_MINIMAL > >>>> return false; > >>>> #else > >>>> return true; > >>>> #endif > >>>> } > >>>> > >>>> Then, in various places: > >>>> > >>>> if (fdt_do_checks() && check_something()) > >>>> return -FDT_ERR_... > >>>> > >>>> I suppose we could add more options, but the main one from my side is > >>>> to remove all sanity / safety checks for minimal code size. > >>> > >>> Seems reasonable. > >> > >> > >> Are you pushing the updated dtc any time soon then? > > > > I just did. > > > I was rather asking about dtc submodule update in qemu, it is not there yet: > > [vpl1 qemu]$ git submodule status dtc > e54388015af1fb4bf04d0bca99caba1074d9cc42 dtc (v1.4.6) Oh, right. Any particular things you're looking for in it? -- 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
