From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew F. Davis Date: Mon, 5 Dec 2016 16:37:20 -0600 Subject: [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE In-Reply-To: References: <20161114191419.14214-1-afd@ti.com> <20161114191419.14214-2-afd@ti.com> <939d5239-8f35-bb50-fa0c-adc213da833e@ti.com> Message-ID: <2c0f2a9d-a191-135c-88b4-f569421deeab@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/14/2016 06:33 PM, Simon Glass wrote: > Hi Andrew, > > On 14 November 2016 at 15:05, Andrew F. Davis wrote: >> On 11/14/2016 02:44 PM, Simon Glass wrote: >>> Hi Andrew, >>> >>> On 14 November 2016 at 12:14, Andrew F. Davis wrote: >>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>> this will abort image loading if the image is not a FIT image. >>>> >>>> Signed-off-by: Andrew F. Davis >>>> --- >>>> Kconfig | 9 +++++++++ >>>> common/spl/spl.c | 5 +++++ >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/Kconfig b/Kconfig >>>> index 1263d0b..eefebef 100644 >>>> --- a/Kconfig >>>> +++ b/Kconfig >>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>> processed before being added to the FIT image). >>>> >>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>> >>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>> boot is disabled. >>> >> >> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >> based. If we only disable legacy image support then RAW images should >> still be allowed, but we will fail early anyway, we will start to need >> an unmaintainable amount of pre-processor logic to to handle the >> different image types and what is allowed/not allowed. >> >> Even worse some boot modes don't seem to support FIT images (net, >> onenand) so these will alway expect legacy to work. Right now we simply >> have to disable these modes. > > IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to > fit in with the legacy format. Otherwise we'll get very confused I > think. > I'm not sure what you are suggesting here, would you like CONFIG_SPL_SUPPORT_RAW_IMAGE CONFIG_SPL_SUPPORT_LEGACY_IMAGE CONFIG_SPL_SUPPORT_FIT_IMAGE And then we disable as needed? I'm not sure this will work in our case, as a new image type may be introduced and enabled by default, this will break our board security until we discover this and disabled it. The benefit of a negative option for us is that we can specify we *only* allow FIT, then it will be obvious to someone adding a new image type they will not meet this check and should not put code outside this block. >> >>>> + bool "Disable SPL loading of non-FIT images" >>>> + default y if SPL_FIT_SIGNATURE >>>> + help >>>> + SPL will not load and image if it is not a FIT image. This is >>>> + useful for devices that only support authentication/encryption >>>> + through SPL FIT loading paths and do not want SPL falling back >>>> + to legacy image loading when a non-FIT image is present. >>>> + >>>> config SPL_DFU_SUPPORT >>>> bool "Enable SPL with DFU to load binaries to memory device" >>>> depends on USB >>>> diff --git a/common/spl/spl.c b/common/spl/spl.c >>>> index bdb165a..3d8bee9 100644 >>>> --- a/common/spl/spl.c >>>> +++ b/common/spl/spl.c >>>> @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) >>>> int spl_parse_image_header(struct spl_image_info *spl_image, >>>> const struct image_header *header) >>>> { >>>> +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE >>>> + /* non-FIT image found, proceed to other boot methods. */ >>>> + return -EINVAL; >>> >>> How about -EPROTONOSUPPORT since the request is not really invalid. >>> >>>> +#else >>>> u32 header_size = sizeof(struct image_header); >>>> >>>> if (image_get_magic(header) == IH_MAGIC) { >>>> @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, >>>> spl_set_header_raw_uboot(spl_image); >>>> #endif >>>> } >>>> +#endif >>>> return 0; >>>> } >>>> >>>> -- >>>> 2.10.1 >>>> > > Regards, > Simon >