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) -- Alexey -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
