On Thu, Oct 24, 2019 at 09:29:22PM -0600, Simon Glass wrote:
> Allow enabling FDT_ASSUME_SANE to disable basic checks.
>
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> libfdt/fdt.c | 35 ++++++++++++++++++++---------------
> libfdt/fdt_ro.c | 2 +-
> libfdt/fdt_rw.c | 22 +++++++++++++++++++---
> libfdt/fdt_sw.c | 13 ++++++++-----
> libfdt/libfdt_internal.h | 7 +++++--
> 5 files changed, 53 insertions(+), 26 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index b706e42..c82c2aa 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -82,20 +82,23 @@ int fdt_check_header(const void *fdt)
>
> if (fdt_magic(fdt) != FDT_MAGIC)
> return -FDT_ERR_BADMAGIC;
> - hdrsize = fdt_header_size(fdt);
> if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> || (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION))
> return -FDT_ERR_BADVERSION;
> if (fdt_version(fdt) < fdt_last_comp_version(fdt))
> return -FDT_ERR_BADVERSION;
> + if (fdt_chk_basic()) {
> + hdrsize = fdt_header_size(fdt);
>
> - if ((fdt_totalsize(fdt) < hdrsize)
> - || (fdt_totalsize(fdt) > INT_MAX))
> - return -FDT_ERR_TRUNCATED;
> + if ((fdt_totalsize(fdt) < hdrsize)
> + || (fdt_totalsize(fdt) > INT_MAX))
> + return -FDT_ERR_TRUNCATED;
>
> - /* Bounds check memrsv block */
> - if (!check_off_(hdrsize, fdt_totalsize(fdt), fdt_off_mem_rsvmap(fdt)))
> - return -FDT_ERR_TRUNCATED;
> + /* Bounds check memrsv block */
> + if (!check_off_(hdrsize, fdt_totalsize(fdt),
> + fdt_off_mem_rsvmap(fdt)))
> + return -FDT_ERR_TRUNCATED;
> + }
>
> /* Bounds check structure block */
> if (fdt_version(fdt) < 17) {
> @@ -121,10 +124,11 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
> {
> unsigned absoffset = offset + fdt_off_dt_struct(fdt);
>
> - if ((absoffset < offset)
> - || ((absoffset + len) < absoffset)
> - || (absoffset + len) > fdt_totalsize(fdt))
> - return NULL;
> + if (fdt_chk_basic())
> + if ((absoffset < offset)
> + || ((absoffset + len) < absoffset)
> + || (absoffset + len) > fdt_totalsize(fdt))
> + return NULL;
>
> if (fdt_version(fdt) >= 0x11)
> if (((offset + len) < offset)
> @@ -143,7 +147,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>
> *nextoffset = -FDT_ERR_TRUNCATED;
> tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE);
> - if (!tagp)
> + if (fdt_chk_basic() && !tagp)
> return FDT_END; /* premature end */
> tag = fdt32_to_cpu(*tagp);
> offset += FDT_TAGSIZE;
> @@ -155,13 +159,13 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> do {
> p = fdt_offset_ptr(fdt, offset++, 1);
> } while (p && (*p != '\0'));
> - if (!p)
> + if (fdt_chk_basic() && !p)
> return FDT_END; /* premature end */
> break;
>
> case FDT_PROP:
> lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
> - if (!lenp)
> + if (fdt_chk_basic() && !lenp)
> return FDT_END; /* premature end */
> /* skip-name offset, length and value */
> offset += sizeof(struct fdt_property) - FDT_TAGSIZE
> @@ -180,7 +184,8 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> return FDT_END;
> }
>
> - if (!fdt_offset_ptr(fdt, startoffset, offset - startoffset))
> + if (fdt_chk_basic() &&
> + !fdt_offset_ptr(fdt, startoffset, offset - startoffset))
> return FDT_END; /* premature end */
>
> *nextoffset = FDT_TAGALIGN(offset);
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index a5c2797..fa699aa 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -346,7 +346,7 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt,
> int err;
> const struct fdt_property *prop;
>
> - if ((err = fdt_check_prop_offset_(fdt, offset)) < 0) {
> + if (fdt_chk_basic() && (err = fdt_check_prop_offset_(fdt, offset)) < 0) {
> if (lenp)
> *lenp = err;
> return NULL;
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 8795947..d3750f5 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -13,6 +13,8 @@
> static int fdt_blocks_misordered_(const void *fdt,
> int mem_rsv_size, int struct_size)
> {
> + if (!fdt_chk_basic())
> + return false;
> return (fdt_off_mem_rsvmap(fdt) < FDT_ALIGN(sizeof(struct fdt_header), 8))
> || (fdt_off_dt_struct(fdt) <
> (fdt_off_mem_rsvmap(fdt) + mem_rsv_size))
> @@ -24,6 +26,8 @@ static int fdt_blocks_misordered_(const void *fdt,
>
> static int fdt_rw_probe_(void *fdt)
> {
> + if (!fdt_chk_basic())
> + return 0;
> FDT_RO_PROBE(fdt);
>
> if (fdt_version(fdt) < 17)
> @@ -112,6 +116,15 @@ static int fdt_splice_string_(void *fdt, int newlen)
> return 0;
> }
>
> +/**
> + * fdt_find_add_string_() - Find or allocate a string
> + *
> + * @fdt: pointer to the device tree to check/adjust
> + * @s: string to find/add
> + * @allocated: Set to 0 if the string was found, 1 if not found and so
> + * allocated. Ignored if !fdt_chk_basic()
> + * @return offset of string in the string table (whether found or added)
> + */
> static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
> {
> char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
> @@ -120,7 +133,8 @@ static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
> int len = strlen(s) + 1;
> int err;
>
> - *allocated = 0;
> + if (fdt_chk_basic())
> + *allocated = 0;
Putting the setting of *allocated behind the gate requires a comment I
think - with ASSUME_SANE enabled, this means that *allocated will not
be initialized by this function which could easily be a gotcha.
>
> p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
> if (p)
> @@ -132,7 +146,8 @@ static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
> if (err)
> return err;
>
> - *allocated = 1;
> + if (fdt_chk_basic())
> + *allocated = 1;
>
> memcpy(new, s, len);
> return (new - strtab);
> @@ -206,7 +221,8 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
>
> err = fdt_splice_struct_(fdt, *prop, 0, proplen);
> if (err) {
> - if (allocated)
> + /* Delete the string if we failed to add it */
> + if (fdt_chk_basic() && allocated)
> fdt_del_last_string_(fdt, name);
This doesn't seem like an obvious change to go into this asusmption
option. IIRC this can fail because we ran out of space in the buffer,
which isn't really in keeping with the other assumptions we're making
here - that can happen even with a correct tree and correct arguments
to all the functions.
> return err;
> }
> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
> index 76bea22..c4c7de7 100644
> --- a/libfdt/fdt_sw.c
> +++ b/libfdt/fdt_sw.c
> @@ -12,17 +12,20 @@
>
> static int fdt_sw_probe_(void *fdt)
> {
> - if (fdt_magic(fdt) == FDT_MAGIC)
> - return -FDT_ERR_BADSTATE;
> - else if (fdt_magic(fdt) != FDT_SW_MAGIC)
> - return -FDT_ERR_BADMAGIC;
> + if (fdt_chk_basic()) {
> + if (fdt_magic(fdt) == FDT_MAGIC)
> + return -FDT_ERR_BADSTATE;
> + else if (fdt_magic(fdt) != FDT_SW_MAGIC)
> + return -FDT_ERR_BADMAGIC;
> + }
> +
> return 0;
> }
>
> #define FDT_SW_PROBE(fdt) \
> { \
> int err; \
> - if ((err = fdt_sw_probe_(fdt)) != 0) \
> + if (fdt_chk_basic() && (err = fdt_sw_probe_(fdt)) != 0) \
> return err; \
> }
>
> diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
> index 33b6cd3..5436e2c 100644
> --- a/libfdt/libfdt_internal.h
> +++ b/libfdt/libfdt_internal.h
> @@ -14,8 +14,11 @@ int fdt_ro_probe_(const void *fdt);
> #define FDT_RO_PROBE(fdt) \
> { \
> int totalsize_; \
> - if ((totalsize_ = fdt_ro_probe_(fdt)) < 0) \
> - return totalsize_; \
> + if (fdt_chk_basic()) { \
> + totalsize_ = fdt_ro_probe_(fdt); \
> + if (totalsize_ < 0) \
> + return totalsize_; \
> + } \
> }
>
> int fdt_check_node_offset_(const void *fdt, int offset);
--
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
