On 02/10/2017 10:23 AM, Simon Glass wrote: > Hi Andrew, > > On 8 February 2017 at 08:18, Andrew F. Davis <a...@ti.com> wrote: >> On 12/06/2016 09:47 PM, Simon Glass wrote: >>> Hi Andrew, >>> >>> On 5 December 2016 at 17:37, Andrew F. Davis <a...@ti.com> wrote: >>>> On 11/14/2016 06:33 PM, Simon Glass wrote: >>>>> Hi Andrew, >>>>> >>>>> On 14 November 2016 at 15:05, Andrew F. Davis <a...@ti.com> wrote: >>>>>> On 11/14/2016 02:44 PM, Simon Glass wrote: >>>>>>> Hi Andrew, >>>>>>> >>>>>>> On 14 November 2016 at 12:14, Andrew F. Davis <a...@ti.com> 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 <a...@ti.com> >>>>>>>> --- >>>>>>>> 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 positivoption only case >> would be a bunch of checks that all other image modes are off, then >> block undefining the same code I am here. > > But why is SPL different from U-Boot proper on this point? It seems > like we could use the same scheme in SPL as we do in U-Boot proper? > > Positive options are easier to understand and combine. If we end up > adding another image format it wouldn't be hard to default it to n if > we are using secure boot. >
Right after I send this response I caved and decided to do it your way in v3: https://www.mail-archive.com/u-boot@lists.denx.de/msg238520.html Sorry I forgot to express that here. Thanks, Andrew > Regards, > Simon > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
