From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew F. Davis Date: Wed, 8 Feb 2017 09:18:22 -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> <2c0f2a9d-a191-135c-88b4-f569421deeab@ti.com> Message-ID: <98948698-40c4-c8aa-af4e-9d050104c4ec@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 12/06/2016 09:47 PM, Simon Glass wrote: > Hi Andrew, > > On 5 December 2016 at 17:37, Andrew F. Davis wrote: >> 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. > > I don't think we add new image types often, and they should default to > n just as we do for U-Boot proper. Although something odd has happened > with DISABLE_IMAGE_LEGACY. > > his should of thing should be caught in a security review. > > Also you could add something to print a warning if secure boot is > enabled but insecure boot options are available? Perhaps that should > be another option, and default to y? > > It's just that it is really hard to deal with multiple negative > options, so best avoided if we can. > I agree in general, but this time I think this is hard to properly avoid. All that would be different with a positive option only case would be a bunch of checks that all other image modes are off, then block undefining the same code I am here. Andrew > Regards, > Simon >