| Message ID | 20210506142438.1310977-20-sjg@chromium.org |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | image: Reduce #ifdefs and ad-hoc defines in image code | expand |
On 5/6/21 9:24 AM, Simon Glass wrote: > Drop the #ifdefs which are easy to remove without refactoring. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > common/Kconfig.boot | 10 ++++++++++ > common/image-fit-sig.c | 8 ++------ > common/image-fit.c | 7 ++++--- > 3 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/common/Kconfig.boot b/common/Kconfig.boot > index 03a6e6f214f..a31d9847124 100644 > --- a/common/Kconfig.boot > +++ b/common/Kconfig.boot > @@ -191,6 +191,16 @@ config SPL_FIT_SIGNATURE > select SPL_IMAGE_SIGN_INFO > select SPL_FIT_FULL_CHECK > > +config SPL_FIT_SIGNATURE_MAX_SIZE > + hex "Max size of signed FIT structures in SPL" > + depends on SPL_FIT_SIGNATURE > + default 0x10000000 > + help > + This option sets a max size in bytes for verified FIT uImages. > + A sane value of 256MB protects corrupted DTB structures from overlapping > + device memory. Assure this size does not extend past expected storage > + space. > + I can't find an argument of why we'd want a separate FIT_SIGNATURE_MAX_SIZE for SPL. This also seems unrelated to the commit message of reducing ifdefs. > config SPL_LOAD_FIT > bool "Enable SPL loading U-Boot as a FIT (basic fitImage features)" > select SPL_FIT > diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c > index 12a6745c642..22f89861048 100644 > --- a/common/image-fit-sig.c > +++ b/common/image-fit-sig.c > @@ -49,10 +49,8 @@ struct image_region *fit_region_make_list(const void *fit, > * Use malloc() except in SPL (to save code size). In SPL the caller > * must allocate the array. > */ > -#ifndef CONFIG_SPL_BUILD > - if (!region) > + if (!IS_ENABLED(CONFIG_SPL_BUILD) && !region) > region = calloc(sizeof(*region), count); > -#endif > if (!region) > return NULL; > for (i = 0; i < count; i++) { > @@ -72,12 +70,10 @@ static int fit_image_setup_verify(struct image_sign_info *info, > char *algo_name; > const char *padding_name; > > -#ifndef USE_HOSTCC > - if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) { > + if (fdt_totalsize(fit) > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)) { > *err_msgp = "Total size too large"; > return 1; > } > -#endif > if (fit_image_hash_get_algo(fit, noffset, &algo_name)) { > *err_msgp = "Can't get hash algo property"; > return -1; > diff --git a/common/image-fit.c b/common/image-fit.c > index f8aa61fc99d..882e872144f 100644 > --- a/common/image-fit.c > +++ b/common/image-fit.c > @@ -1993,9 +1993,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, > int type_ok, os_ok; > ulong load, load_end, data, len; > uint8_t os, comp; > -#ifndef USE_HOSTCC > - uint8_t os_arch; > -#endif > const char *prop_name; > int ret; > > @@ -2087,8 +2084,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr, > } > > #ifndef USE_HOSTCC > + { > + uint8_t os_arch; > + > fit_image_get_arch(fit, noffset, &os_arch); > images->os.arch = os_arch; > + } > #endif > > bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL); >
Hi Alex, On Fri, 14 May 2021 at 15:50, Alex G. <mr.nuke.me@gmail.com> wrote: > > > > On 5/6/21 9:24 AM, Simon Glass wrote: > > Drop the #ifdefs which are easy to remove without refactoring. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > (no changes since v1) > > > > common/Kconfig.boot | 10 ++++++++++ > > common/image-fit-sig.c | 8 ++------ > > common/image-fit.c | 7 ++++--- > > 3 files changed, 16 insertions(+), 9 deletions(-) > > > > diff --git a/common/Kconfig.boot b/common/Kconfig.boot > > index 03a6e6f214f..a31d9847124 100644 > > --- a/common/Kconfig.boot > > +++ b/common/Kconfig.boot > > @@ -191,6 +191,16 @@ config SPL_FIT_SIGNATURE > > select SPL_IMAGE_SIGN_INFO > > select SPL_FIT_FULL_CHECK > > > > +config SPL_FIT_SIGNATURE_MAX_SIZE > > + hex "Max size of signed FIT structures in SPL" > > + depends on SPL_FIT_SIGNATURE > > + default 0x10000000 > > + help > > + This option sets a max size in bytes for verified FIT uImages. > > + A sane value of 256MB protects corrupted DTB structures from overlapping > > + device memory. Assure this size does not extend past expected storage > > + space. > > + > > I can't find an argument of why we'd want a separate > FIT_SIGNATURE_MAX_SIZE for SPL. This also seems unrelated to the commit > message of reducing ifdefs. Often SPL has lower limits, e.g. since there is only so much SRAM, a large file might indicate some sort of attack. Regards, SImon
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 03a6e6f214f..a31d9847124 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -191,6 +191,16 @@ config SPL_FIT_SIGNATURE select SPL_IMAGE_SIGN_INFO select SPL_FIT_FULL_CHECK +config SPL_FIT_SIGNATURE_MAX_SIZE + hex "Max size of signed FIT structures in SPL" + depends on SPL_FIT_SIGNATURE + default 0x10000000 + help + This option sets a max size in bytes for verified FIT uImages. + A sane value of 256MB protects corrupted DTB structures from overlapping + device memory. Assure this size does not extend past expected storage + space. + config SPL_LOAD_FIT bool "Enable SPL loading U-Boot as a FIT (basic fitImage features)" select SPL_FIT diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c index 12a6745c642..22f89861048 100644 --- a/common/image-fit-sig.c +++ b/common/image-fit-sig.c @@ -49,10 +49,8 @@ struct image_region *fit_region_make_list(const void *fit, * Use malloc() except in SPL (to save code size). In SPL the caller * must allocate the array. */ -#ifndef CONFIG_SPL_BUILD - if (!region) + if (!IS_ENABLED(CONFIG_SPL_BUILD) && !region) region = calloc(sizeof(*region), count); -#endif if (!region) return NULL; for (i = 0; i < count; i++) { @@ -72,12 +70,10 @@ static int fit_image_setup_verify(struct image_sign_info *info, char *algo_name; const char *padding_name; -#ifndef USE_HOSTCC - if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) { + if (fdt_totalsize(fit) > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)) { *err_msgp = "Total size too large"; return 1; } -#endif if (fit_image_hash_get_algo(fit, noffset, &algo_name)) { *err_msgp = "Can't get hash algo property"; return -1; diff --git a/common/image-fit.c b/common/image-fit.c index f8aa61fc99d..882e872144f 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1993,9 +1993,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr, int type_ok, os_ok; ulong load, load_end, data, len; uint8_t os, comp; -#ifndef USE_HOSTCC - uint8_t os_arch; -#endif const char *prop_name; int ret; @@ -2087,8 +2084,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr, } #ifndef USE_HOSTCC + { + uint8_t os_arch; + fit_image_get_arch(fit, noffset, &os_arch); images->os.arch = os_arch; + } #endif bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
Drop the #ifdefs which are easy to remove without refactoring. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) common/Kconfig.boot | 10 ++++++++++ common/image-fit-sig.c | 8 ++------ common/image-fit.c | 7 ++++--- 3 files changed, 16 insertions(+), 9 deletions(-)