From: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
To: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Devicetree Compiler
<devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v5 3/7] libfdt: Add support for disabling dtb checks
Date: Mon, 10 Feb 2020 15:02:01 +1100 [thread overview]
Message-ID: <20200210040201.GA22584@umbus.fritz.box> (raw)
In-Reply-To: <20200206054034.162732-4-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 9142 bytes --]
On Wed, Feb 05, 2020 at 10:40:30PM -0700, Simon Glass wrote:
> Support ASSUME_VALID_DTB to disable some sanity checks
>
> If we assume that the DTB itself is valid then we can skip some checks and
> save code space. Add various conditions to handle this.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v5:
> - Split out VALID_DTB checks into a separate patch
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> libfdt/fdt.c | 50 ++++++++++++++++++++++------------------
> libfdt/fdt_ro.c | 7 ++++--
> libfdt/fdt_rw.c | 7 +++++-
> libfdt/fdt_sw.c | 30 ++++++++++++++++--------
> libfdt/libfdt_internal.h | 7 ++++--
> 5 files changed, 64 insertions(+), 37 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 3e37a4b..03f2b7d 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -81,38 +81,44 @@ 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;
> + hdrsize = fdt_header_size(fdt);
> + if (!can_assume(VALID_DTB)) {
>
> - 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;
> + if ((fdt_totalsize(fdt) < hdrsize)
> + || (fdt_totalsize(fdt) > INT_MAX))
> + return -FDT_ERR_TRUNCATED;
>
> - /* Bounds check structure block */
> - if (fdt_version(fdt) < 17) {
> + /* Bounds check memrsv block */
> if (!check_off_(hdrsize, fdt_totalsize(fdt),
> - fdt_off_dt_struct(fdt)))
> + fdt_off_mem_rsvmap(fdt)))
> return -FDT_ERR_TRUNCATED;
> - } else {
> + }
> +
> + if (!can_assume(VALID_DTB)) {
> + /* Bounds check structure block */
> + if (fdt_version(fdt) < 17) {
> + if (!check_off_(hdrsize, fdt_totalsize(fdt),
> + fdt_off_dt_struct(fdt)))
> + return -FDT_ERR_TRUNCATED;
> + } else {
> + if (!check_block_(hdrsize, fdt_totalsize(fdt),
> + fdt_off_dt_struct(fdt),
> + fdt_size_dt_struct(fdt)))
> + return -FDT_ERR_TRUNCATED;
> + }
> +
> + /* Bounds check strings block */
> if (!check_block_(hdrsize, fdt_totalsize(fdt),
> - fdt_off_dt_struct(fdt),
> - fdt_size_dt_struct(fdt)))
> + fdt_off_dt_strings(fdt),
> + fdt_size_dt_strings(fdt)))
> return -FDT_ERR_TRUNCATED;
> }
>
> - /* Bounds check strings block */
> - if (!check_block_(hdrsize, fdt_totalsize(fdt),
> - fdt_off_dt_strings(fdt), fdt_size_dt_strings(fdt)))
> - return -FDT_ERR_TRUNCATED;
> -
> return 0;
> }
>
> @@ -142,7 +148,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 (!can_assume(VALID_DTB) && !tagp)
> return FDT_END; /* premature end */
> tag = fdt32_to_cpu(*tagp);
> offset += FDT_TAGSIZE;
> @@ -154,13 +160,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 (!can_assume(VALID_DTB) && !p)
> return FDT_END; /* premature end */
> break;
>
> case FDT_PROP:
> lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
> - if (!lenp)
> + if (!can_assume(VALID_DTB) && !lenp)
> return FDT_END; /* premature end */
> /* skip-name offset, length and value */
> offset += sizeof(struct fdt_property) - FDT_TAGSIZE
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index a5c2797..b41083f 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -289,9 +289,12 @@ const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
> const char *nameptr;
> int err;
>
> - if (((err = fdt_ro_probe_(fdt)) < 0)
> - || ((err = fdt_check_node_offset_(fdt, nodeoffset)) < 0))
> + if (!can_assume(VALID_DTB)) {
> + if ((err = fdt_ro_probe_(fdt)) < 0)
Seems like it might be cleaner to include the can_assume() check into
fdt_ro_probe_().
> goto fail;
> + if ((err = fdt_check_node_offset_(fdt, nodeoffset)) < 0)
> + goto fail;
> + }
>
> nameptr = nh->name;
>
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 8795947..7a41a31 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 (can_assume(VALID_DTB))
> + return false;
Urgh... the spec doesn't actually specify a block order, so I'm not
sure we can put this one under VALID_DTB.
> 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 (can_assume(VALID_DTB))
> + return 0;
> FDT_RO_PROBE(fdt);
>
> if (fdt_version(fdt) < 17)
> @@ -40,7 +44,8 @@ static int fdt_rw_probe_(void *fdt)
> #define FDT_RW_PROBE(fdt) \
> { \
> int err_; \
> - if ((err_ = fdt_rw_probe_(fdt)) != 0) \
> + if (!can_assume(VALID_DTB) && \
> + (err_ = fdt_rw_probe_(fdt)) != 0) \
You shouldn't need the test both inside fdt_rw_probe_() and in the
FDT_RW_PROBE() macro, no?
> return err_; \
> }
>
> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
> index 76bea22..8fca816 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 (!can_assume(VALID_DTB)) {
> + 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 (!can_assume(VALID_DTB) && (err = fdt_sw_probe_(fdt)) != 0) \
Same question here.
> return err; \
> }
>
> @@ -38,7 +41,7 @@ static int fdt_sw_probe_memrsv_(void *fdt)
> if (err)
> return err;
>
> - if (fdt_off_dt_strings(fdt) != 0)
> + if (!can_assume(VALID_DTB) && fdt_off_dt_strings(fdt) != 0)
> return -FDT_ERR_BADSTATE;
> return 0;
> }
> @@ -46,7 +49,8 @@ static int fdt_sw_probe_memrsv_(void *fdt)
> #define FDT_SW_PROBE_MEMRSV(fdt) \
> { \
> int err; \
> - if ((err = fdt_sw_probe_memrsv_(fdt)) != 0) \
> + if (!can_assume(VALID_DTB) && \
And here.
> + (err = fdt_sw_probe_memrsv_(fdt)) != 0) \
> return err; \
> }
>
> @@ -60,7 +64,11 @@ static int fdt_sw_probe_memrsv_(void *fdt)
> */
> static int fdt_sw_probe_struct_(void *fdt)
> {
> - int err = fdt_sw_probe_(fdt);
> + int err;
> +
> + if (can_assume(VALID_DTB))
> + return 0;
> + err = fdt_sw_probe_(fdt);
And here.
> if (err)
> return err;
>
> @@ -72,7 +80,8 @@ static int fdt_sw_probe_struct_(void *fdt)
> #define FDT_SW_PROBE_STRUCT(fdt) \
> { \
> int err; \
> - if ((err = fdt_sw_probe_struct_(fdt)) != 0) \
> + if (!can_assume(VALID_DTB) && \
> + (err = fdt_sw_probe_struct_(fdt)) != 0) \
And here.
> return err; \
> }
>
> @@ -151,7 +160,8 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
> headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
> tailsize = fdt_size_dt_strings(fdt);
>
> - if ((headsize + tailsize) > fdt_totalsize(fdt))
> + if (!can_assume(VALID_DTB) &&
> + headsize + tailsize > fdt_totalsize(fdt))
> return -FDT_ERR_INTERNAL;
>
> if ((headsize + tailsize) > bufsize)
> diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
> index 1e31d10..1aa732b 100644
> --- a/libfdt/libfdt_internal.h
> +++ b/libfdt/libfdt_internal.h
> @@ -14,8 +14,11 @@ int32_t fdt_ro_probe_(const void *fdt);
> #define FDT_RO_PROBE(fdt) \
> { \
> int32_t totalsize_; \
> - if ((totalsize_ = fdt_ro_probe_(fdt)) < 0) \
> - return totalsize_; \
> + if (!can_assume(VALID_DTB)) { \
> + totalsize_ = fdt_ro_probe_(fdt); \
> + if (totalsize_ < 0) \
> + return totalsize_; \
> + } \
And here.
> }
>
> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-02-10 4:02 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-06 5:40 [PATCH v5 0/7] libfdt: Allow more control of code size Simon Glass
[not found] ` <20200206054034.162732-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2020-02-06 5:40 ` [PATCH v5 1/7] libfdt: De-inline fdt_header_size() Simon Glass
2020-02-06 5:40 ` [PATCH v5 2/7] Add a way to control the level of checks in the code Simon Glass
2020-02-06 5:40 ` [PATCH v5 3/7] libfdt: Add support for disabling dtb checks Simon Glass
[not found] ` <20200206054034.162732-4-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2020-02-10 4:02 ` David Gibson [this message]
[not found] ` <20200210040201.GA22584-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2020-02-11 1:04 ` Simon Glass
[not found] ` <CAPnjgZ3Ut2CCVqDnRiafMqR+sV0eMn_obhcXYLfbuQgXhhPS1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-11 1:42 ` David Gibson
[not found] ` <20200211014254.GJ22584-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2020-02-11 20:08 ` Simon Glass
[not found] ` <CAPnjgZ2M+Y7ExHar2+iCinW+w8DniNUFn=_VRs+-_+zfRuoG7Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-12 0:53 ` David Gibson
[not found] ` <20200212005316.GM22584-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2020-02-12 1:20 ` Simon Glass
2020-02-06 5:40 ` [PATCH v5 4/7] libfdt: Add support for disabling sanity checks Simon Glass
[not found] ` <20200206054034.162732-5-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2020-02-11 1:12 ` David Gibson
[not found] ` <20200211011231.GI22584-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2020-02-11 20:08 ` Simon Glass
[not found] ` <CAPnjgZ29PgXrG65RKVsGFzxiSYX5VJrynD+u-mwwiFO+HkKQPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-12 1:05 ` David Gibson
[not found] ` <20200212010519.GN22584-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2020-02-12 1:23 ` Simon Glass
[not found] ` <CAPnjgZ1apEQmMYKT0TVQQOGgtUwOHryRbmujO92ovqHjT1rxrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-12 2:05 ` David Gibson
2020-02-06 5:40 ` [PATCH v5 5/7] libfdt: Add support for disabling rollback handling Simon Glass
2020-02-06 5:40 ` [PATCH v5 6/7] libfdt: Add support for disabling version checks Simon Glass
[not found] ` <20200206054034.162732-7-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2020-02-11 3:10 ` David Gibson
2020-02-06 5:40 ` [PATCH v5 7/7] libfdt: Allow exclusion of fdt_check_full() Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200210040201.GA22584@umbus.fritz.box \
--to=david-xt8fgy+axnrb3ne2bgzf6laj5h9x9tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.