Re: [PATCHv2 00/13] Improve libfdt's memory safety

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux