[U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation

[U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation

[Andrew F. Davis]

When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
is exported and the user re-builds, make will detect this file as
unchangedand and so assume it does not need to be re-generated. This
causes it to pack unsigned files. Fix this by not generating these
fake unsigned *_HS files.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 arch/arm/mach-omap2/config_secure.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
index 1122439..33c7059 100644
--- a/arch/arm/mach-omap2/config_secure.mk
+++ b/arch/arm/mach-omap2/config_secure.mk
@@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
 else
 cmd_omapsecureimg = echo "WARNING:" \
 	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
-	"$@ was NOT created!"; cp $< $@
+	"$@ was NOT created!";
 endif
 else
 cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
 	"variable must be defined for TI secure devices." \
-	"$@ was NOT created!"; cp $< $@
+	"$@ was NOT created!";
 endif
 endif
 
-- 
2.10.2

[U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:

> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> is exported and the user re-builds, make will detect this file as
> unchangedand and so assume it does not need to be re-generated. This
> causes it to pack unsigned files. Fix this by not generating these
> fake unsigned *_HS files.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161208/a1752504/attachment.sig>

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:

> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> is exported and the user re-builds, make will detect this file as
> unchangedand and so assume it does not need to be re-generated. This
> causes it to pack unsigned files. Fix this by not generating these
> fake unsigned *_HS files.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---
>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
> index 1122439..33c7059 100644
> --- a/arch/arm/mach-omap2/config_secure.mk
> +++ b/arch/arm/mach-omap2/config_secure.mk
> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
>  else
>  cmd_omapsecureimg = echo "WARNING:" \
>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
> -	"$@ was NOT created!"; cp $< $@
> +	"$@ was NOT created!";
>  endif
>  else
>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
>  	"variable must be defined for TI secure devices." \
> -	"$@ was NOT created!"; cp $< $@
> +	"$@ was NOT created!";
>  endif
>  endif

OK, but now that I build test this (without the tools present) this is a
NAK.  The root problem is that if we don't make that dummy file we then:
       arm:  +   am57xx_hs_evm
+(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
+(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
+(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
+(am57xx_hs_evm) make: *** [sub-make] Error 2

So perhaps we need to make use of some other logic to rebuild on
TI_SECURE_DEV_PKG changing?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161209/7ce87fd0/attachment.sig>

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Andrew F. Davis]

On 12/09/2016 01:59 PM, Tom Rini wrote:
> On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
> 
>> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
>> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
>> is exported and the user re-builds, make will detect this file as
>> unchangedand and so assume it does not need to be re-generated. This
>> causes it to pack unsigned files. Fix this by not generating these
>> fake unsigned *_HS files.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>> ---
>>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
>> index 1122439..33c7059 100644
>> --- a/arch/arm/mach-omap2/config_secure.mk
>> +++ b/arch/arm/mach-omap2/config_secure.mk
>> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
>>  else
>>  cmd_omapsecureimg = echo "WARNING:" \
>>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
>> -	"$@ was NOT created!"; cp $< $@
>> +	"$@ was NOT created!";
>>  endif
>>  else
>>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
>>  	"variable must be defined for TI secure devices." \
>> -	"$@ was NOT created!"; cp $< $@
>> +	"$@ was NOT created!";
>>  endif
>>  endif
> 
> OK, but now that I build test this (without the tools present) this is a
> NAK.  The root problem is that if we don't make that dummy file we then:
>        arm:  +   am57xx_hs_evm
> +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
> +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
> +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
> +(am57xx_hs_evm) make: *** [sub-make] Error 2
> 

Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
defined. You cannot sign images that *need* to be signed to work on this
platform, making a fake un-bootable image instead of failing is a hack
and it confuses the make system when you do put the signing tool in-place.

> So perhaps we need to make use of some other logic to rebuild on
> TI_SECURE_DEV_PKG changing?
> 

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Fri, Dec 09, 2016 at 02:05:29PM -0600, Andrew F. Davis wrote:
> On 12/09/2016 01:59 PM, Tom Rini wrote:
> > On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
> > 
> >> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> >> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> >> is exported and the user re-builds, make will detect this file as
> >> unchangedand and so assume it does not need to be re-generated. This
> >> causes it to pack unsigned files. Fix this by not generating these
> >> fake unsigned *_HS files.
> >>
> >> Signed-off-by: Andrew F. Davis <afd@ti.com>
> >> Reviewed-by: Tom Rini <trini@konsulko.com>
> >> ---
> >>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
> >> index 1122439..33c7059 100644
> >> --- a/arch/arm/mach-omap2/config_secure.mk
> >> +++ b/arch/arm/mach-omap2/config_secure.mk
> >> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
> >>  else
> >>  cmd_omapsecureimg = echo "WARNING:" \
> >>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
> >> -	"$@ was NOT created!"; cp $< $@
> >> +	"$@ was NOT created!";
> >>  endif
> >>  else
> >>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
> >>  	"variable must be defined for TI secure devices." \
> >> -	"$@ was NOT created!"; cp $< $@
> >> +	"$@ was NOT created!";
> >>  endif
> >>  endif
> > 
> > OK, but now that I build test this (without the tools present) this is a
> > NAK.  The root problem is that if we don't make that dummy file we then:
> >        arm:  +   am57xx_hs_evm
> > +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
> > +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
> > +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
> > +(am57xx_hs_evm) make: *** [sub-make] Error 2
> 
> Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
> defined. You cannot sign images that *need* to be signed to work on this
> platform, making a fake un-bootable image instead of failing is a hack
> and it confuses the make system when you do put the signing tool in-place.

Well, I suppose this is a valid question.  I run into it failing as I
(and travis-ci) build all ARM targets.  Maybe we can have the build not
happen (and echo a Warning) and then not invoke mkimage later on if the
env isn't right?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161209/149129ab/attachment.sig>

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Andrew F. Davis]

On 12/09/2016 02:10 PM, Tom Rini wrote:
> On Fri, Dec 09, 2016 at 02:05:29PM -0600, Andrew F. Davis wrote:
>> On 12/09/2016 01:59 PM, Tom Rini wrote:
>>> On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
>>>
>>>> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
>>>> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
>>>> is exported and the user re-builds, make will detect this file as
>>>> unchangedand and so assume it does not need to be re-generated. This
>>>> causes it to pack unsigned files. Fix this by not generating these
>>>> fake unsigned *_HS files.
>>>>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>> ---
>>>>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
>>>> index 1122439..33c7059 100644
>>>> --- a/arch/arm/mach-omap2/config_secure.mk
>>>> +++ b/arch/arm/mach-omap2/config_secure.mk
>>>> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
>>>>  else
>>>>  cmd_omapsecureimg = echo "WARNING:" \
>>>>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
>>>> -	"$@ was NOT created!"; cp $< $@
>>>> +	"$@ was NOT created!";
>>>>  endif
>>>>  else
>>>>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
>>>>  	"variable must be defined for TI secure devices." \
>>>> -	"$@ was NOT created!"; cp $< $@
>>>> +	"$@ was NOT created!";
>>>>  endif
>>>>  endif
>>>
>>> OK, but now that I build test this (without the tools present) this is a
>>> NAK.  The root problem is that if we don't make that dummy file we then:
>>>        arm:  +   am57xx_hs_evm
>>> +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
>>> +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
>>> +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
>>> +(am57xx_hs_evm) make: *** [sub-make] Error 2
>>
>> Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
>> defined. You cannot sign images that *need* to be signed to work on this
>> platform, making a fake un-bootable image instead of failing is a hack
>> and it confuses the make system when you do put the signing tool in-place.
> 
> Well, I suppose this is a valid question.  I run into it failing as I
> (and travis-ci) build all ARM targets.  Maybe we can have the build not
> happen (and echo a Warning) and then not invoke mkimage later on if the
> env isn't right?
> 

For test building you can export TI_SECURE_DEV_PKG to point to a dummy
signing tool which just runs cp $1 $2. For real world building this tool
is needed just as much as the compiler, if you don't have it you will
not build working images, build needs to fail here.

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Fri, Dec 09, 2016 at 02:24:32PM -0600, Andrew F. Davis wrote:
> On 12/09/2016 02:10 PM, Tom Rini wrote:
> > On Fri, Dec 09, 2016 at 02:05:29PM -0600, Andrew F. Davis wrote:
> >> On 12/09/2016 01:59 PM, Tom Rini wrote:
> >>> On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
> >>>
> >>>> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> >>>> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> >>>> is exported and the user re-builds, make will detect this file as
> >>>> unchangedand and so assume it does not need to be re-generated. This
> >>>> causes it to pack unsigned files. Fix this by not generating these
> >>>> fake unsigned *_HS files.
> >>>>
> >>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> >>>> Reviewed-by: Tom Rini <trini@konsulko.com>
> >>>> ---
> >>>>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
> >>>> index 1122439..33c7059 100644
> >>>> --- a/arch/arm/mach-omap2/config_secure.mk
> >>>> +++ b/arch/arm/mach-omap2/config_secure.mk
> >>>> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
> >>>>  else
> >>>>  cmd_omapsecureimg = echo "WARNING:" \
> >>>>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
> >>>> -	"$@ was NOT created!"; cp $< $@
> >>>> +	"$@ was NOT created!";
> >>>>  endif
> >>>>  else
> >>>>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
> >>>>  	"variable must be defined for TI secure devices." \
> >>>> -	"$@ was NOT created!"; cp $< $@
> >>>> +	"$@ was NOT created!";
> >>>>  endif
> >>>>  endif
> >>>
> >>> OK, but now that I build test this (without the tools present) this is a
> >>> NAK.  The root problem is that if we don't make that dummy file we then:
> >>>        arm:  +   am57xx_hs_evm
> >>> +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
> >>> +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
> >>> +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
> >>> +(am57xx_hs_evm) make: *** [sub-make] Error 2
> >>
> >> Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
> >> defined. You cannot sign images that *need* to be signed to work on this
> >> platform, making a fake un-bootable image instead of failing is a hack
> >> and it confuses the make system when you do put the signing tool in-place.
> > 
> > Well, I suppose this is a valid question.  I run into it failing as I
> > (and travis-ci) build all ARM targets.  Maybe we can have the build not
> > happen (and echo a Warning) and then not invoke mkimage later on if the
> > env isn't right?
> 
> For test building you can export TI_SECURE_DEV_PKG to point to a dummy
> signing tool which just runs cp $1 $2. For real world building this tool
> is needed just as much as the compiler, if you don't have it you will
> not build working images, build needs to fail here.

Hmmm, OK.  But can we not automate that based on TI_SECURE_DEV_PKG being
unset?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161209/5984f608/attachment.sig>

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Andrew F. Davis]

On 12/09/2016 03:30 PM, Tom Rini wrote:
> On Fri, Dec 09, 2016 at 02:24:32PM -0600, Andrew F. Davis wrote:
>> On 12/09/2016 02:10 PM, Tom Rini wrote:
>>> On Fri, Dec 09, 2016 at 02:05:29PM -0600, Andrew F. Davis wrote:
>>>> On 12/09/2016 01:59 PM, Tom Rini wrote:
>>>>> On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
>>>>>
>>>>>> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
>>>>>> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
>>>>>> is exported and the user re-builds, make will detect this file as
>>>>>> unchangedand and so assume it does not need to be re-generated. This
>>>>>> causes it to pack unsigned files. Fix this by not generating these
>>>>>> fake unsigned *_HS files.
>>>>>>
>>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>>>> ---
>>>>>>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
>>>>>> index 1122439..33c7059 100644
>>>>>> --- a/arch/arm/mach-omap2/config_secure.mk
>>>>>> +++ b/arch/arm/mach-omap2/config_secure.mk
>>>>>> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
>>>>>>  else
>>>>>>  cmd_omapsecureimg = echo "WARNING:" \
>>>>>>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
>>>>>> -	"$@ was NOT created!"; cp $< $@
>>>>>> +	"$@ was NOT created!";
>>>>>>  endif
>>>>>>  else
>>>>>>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
>>>>>>  	"variable must be defined for TI secure devices." \
>>>>>> -	"$@ was NOT created!"; cp $< $@
>>>>>> +	"$@ was NOT created!";
>>>>>>  endif
>>>>>>  endif
>>>>>
>>>>> OK, but now that I build test this (without the tools present) this is a
>>>>> NAK.  The root problem is that if we don't make that dummy file we then:
>>>>>        arm:  +   am57xx_hs_evm
>>>>> +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
>>>>> +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
>>>>> +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
>>>>> +(am57xx_hs_evm) make: *** [sub-make] Error 2
>>>>
>>>> Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
>>>> defined. You cannot sign images that *need* to be signed to work on this
>>>> platform, making a fake un-bootable image instead of failing is a hack
>>>> and it confuses the make system when you do put the signing tool in-place.
>>>
>>> Well, I suppose this is a valid question.  I run into it failing as I
>>> (and travis-ci) build all ARM targets.  Maybe we can have the build not
>>> happen (and echo a Warning) and then not invoke mkimage later on if the
>>> env isn't right?
>>
>> For test building you can export TI_SECURE_DEV_PKG to point to a dummy
>> signing tool which just runs cp $1 $2. For real world building this tool
>> is needed just as much as the compiler, if you don't have it you will
>> not build working images, build needs to fail here.
> 
> Hmmm, OK.  But can we not automate that based on TI_SECURE_DEV_PKG being
> unset?
> 

That is what we already do, if TI_SECURE_DEV_PKG is unset, build should
fail, but right now it fakes a successful build, most likely to keep the
auto-validation happy as it does not have the signing tool.

The only other thing I can think of is to always try to sign the images,
even when they have not changed on disk since the last build. Would this
be acceptable?

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:

> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> is exported and the user re-builds, make will detect this file as
> unchangedand and so assume it does not need to be re-generated. This
> causes it to pack unsigned files. Fix this by not generating these
> fake unsigned *_HS files.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161212/4541457f/attachment.sig>

[U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Fri, Dec 09, 2016 at 03:39:34PM -0600, Andrew F. Davis wrote:
> On 12/09/2016 03:30 PM, Tom Rini wrote:
> > On Fri, Dec 09, 2016 at 02:24:32PM -0600, Andrew F. Davis wrote:
> >> On 12/09/2016 02:10 PM, Tom Rini wrote:
> >>> On Fri, Dec 09, 2016 at 02:05:29PM -0600, Andrew F. Davis wrote:
> >>>> On 12/09/2016 01:59 PM, Tom Rini wrote:
> >>>>> On Thu, Dec 08, 2016 at 04:48:07PM -0600, Andrew F. Davis wrote:
> >>>>>
> >>>>>> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> >>>>>> not generated but generate an unsigned one anyway. When TI_SECURE_DEV_PKG
> >>>>>> is exported and the user re-builds, make will detect this file as
> >>>>>> unchangedand and so assume it does not need to be re-generated. This
> >>>>>> causes it to pack unsigned files. Fix this by not generating these
> >>>>>> fake unsigned *_HS files.
> >>>>>>
> >>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> >>>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
> >>>>>> ---
> >>>>>>  arch/arm/mach-omap2/config_secure.mk | 4 ++--
> >>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
> >>>>>> index 1122439..33c7059 100644
> >>>>>> --- a/arch/arm/mach-omap2/config_secure.mk
> >>>>>> +++ b/arch/arm/mach-omap2/config_secure.mk
> >>>>>> @@ -35,12 +35,12 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
> >>>>>>  else
> >>>>>>  cmd_omapsecureimg = echo "WARNING:" \
> >>>>>>  	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
> >>>>>> -	"$@ was NOT created!"; cp $< $@
> >>>>>> +	"$@ was NOT created!";
> >>>>>>  endif
> >>>>>>  else
> >>>>>>  cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
> >>>>>>  	"variable must be defined for TI secure devices." \
> >>>>>> -	"$@ was NOT created!"; cp $< $@
> >>>>>> +	"$@ was NOT created!";
> >>>>>>  endif
> >>>>>>  endif
> >>>>>
> >>>>> OK, but now that I build test this (without the tools present) this is a
> >>>>> NAK.  The root problem is that if we don't make that dummy file we then:
> >>>>>        arm:  +   am57xx_hs_evm
> >>>>> +(am57xx_hs_evm) ./tools/mkimage: Can't open u-boot-nodtb_HS.bin: No such file or directory
> >>>>> +(am57xx_hs_evm) ./tools/mkimage: failed to build FIT
> >>>>> +(am57xx_hs_evm) make[1]: *** [u-boot_HS.img] Error 1
> >>>>> +(am57xx_hs_evm) make: *** [sub-make] Error 2
> >>>>
> >>>> Is this not okay? build *should* fail if TI_SECURE_DEV_PKG is not
> >>>> defined. You cannot sign images that *need* to be signed to work on this
> >>>> platform, making a fake un-bootable image instead of failing is a hack
> >>>> and it confuses the make system when you do put the signing tool in-place.
> >>>
> >>> Well, I suppose this is a valid question.  I run into it failing as I
> >>> (and travis-ci) build all ARM targets.  Maybe we can have the build not
> >>> happen (and echo a Warning) and then not invoke mkimage later on if the
> >>> env isn't right?
> >>
> >> For test building you can export TI_SECURE_DEV_PKG to point to a dummy
> >> signing tool which just runs cp $1 $2. For real world building this tool
> >> is needed just as much as the compiler, if you don't have it you will
> >> not build working images, build needs to fail here.
> > 
> > Hmmm, OK.  But can we not automate that based on TI_SECURE_DEV_PKG being
> > unset?
> > 
> 
> That is what we already do, if TI_SECURE_DEV_PKG is unset, build should
> fail, but right now it fakes a successful build, most likely to keep the
> auto-validation happy as it does not have the signing tool.
> 
> The only other thing I can think of is to always try to sign the images,
> even when they have not changed on disk since the last build. Would this
> be acceptable?

Along with a comment above it about why we always re-sign, yes, lets go
that route.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161212/e6404776/attachment.sig>

[U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation

[Andrew F. Davis]

When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
not generated but generate an unsigned one anyway, first fix this
warning to say that it was generated but not secured.

When the user then exports TI_SECURE_DEV_PKG after getting this warning,
and tries to re-build, 'make' will detect the build artifacts as
unchanged and so assume they do not need to be re-generated. This causes
it to fail to sign the files and it will pack unsigned files into the
final image, even though TI_SECURE_DEV_PKG is now correctly defined and
working.

Fix this by using FORCE on the targets causes them to be re-run even if
the dependent files have not changed.

This then causes another issue. We currently rename the signed dtb files
to overwrite the non-signed ones. We do this so the 'mkimage' tool gives
the packaged dtb sections the correct name. If we do not rename the files
then SPL will not find them during boot.

Fix this by renaming the dtb files by appending _HS to the end of the
filename, after the ".dtb", this causes them to still be named correctly
in the FIT blob.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 arch/arm/mach-omap2/config_secure.mk | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/config_secure.mk b/arch/arm/mach-omap2/config_secure.mk
index 1122439e38..0c843338d7 100644
--- a/arch/arm/mach-omap2/config_secure.mk
+++ b/arch/arm/mach-omap2/config_secure.mk
@@ -3,7 +3,7 @@
 #
 # SPDX-License-Identifier:	GPL-2.0+
 #
-quiet_cmd_mkomapsecimg = MKIMAGE $@
+quiet_cmd_mkomapsecimg = SECURE  $@
 ifneq ($(TI_SECURE_DEV_PKG),)
 ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh),)
 ifneq ($(CONFIG_SPL_BUILD),)
@@ -18,11 +18,12 @@ endif
 else
 cmd_mkomapsecimg = echo "WARNING:" \
 	"$(TI_SECURE_DEV_PKG)/scripts/create-boot-image.sh not found." \
-	"$@ was NOT created!"
+	"$@ was NOT secured!"; cp $< $@
 endif
 else
 cmd_mkomapsecimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
-	"variable must be defined for TI secure devices. $@ was NOT created!"
+	"variable must be defined for TI secure devices. \
+	$@ was NOT secured!"; cp $< $@
 endif
 
 ifdef CONFIG_SPL_LOAD_FIT
@@ -35,51 +36,51 @@ cmd_omapsecureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
 else
 cmd_omapsecureimg = echo "WARNING:" \
 	"$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
-	"$@ was NOT created!"; cp $< $@
+	"$@ was NOT secured!"; cp $< $@
 endif
 else
 cmd_omapsecureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
 	"variable must be defined for TI secure devices." \
-	"$@ was NOT created!"; cp $< $@
+	"$@ was NOT secured!"; cp $< $@
 endif
 endif
 
 
 # Standard X-LOADER target (QPSI, NOR flash)
-u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_X-LOADER: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # For MLO targets (SD card boot) the final file name that is copied to the SD
 # card FAT partition must be MLO, so we make a copy of the output file to a new
 # file with that name
-u-boot-spl_HS_MLO: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_MLO: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 	@if [ -f $@ ]; then \
 		cp -f $@ MLO; \
 	fi
 
 # Standard 2ND target (certain peripheral boot modes)
-u-boot-spl_HS_2ND: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_2ND: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # Standard ULO target (certain peripheral boot modes)
-u-boot-spl_HS_ULO: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_ULO: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # Standard ISSW target (certain devices, various boot modes)
-u-boot-spl_HS_ISSW: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_ISSW: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # For SPI flash on AM335x and AM43xx, these require special byte swap handling
 # so we use the SPI_X-LOADER target instead of X-LOADER and let the
 # create-boot-image.sh script handle that
-u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin
+u-boot-spl_HS_SPI_X-LOADER: $(obj)/u-boot-spl.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # For supporting single stage XiP QSPI on AM43xx, the image is a full u-boot
 # file, not an SPL. In this case the mkomapsecimg command looks for a
 # u-boot-HS_* prefix
-u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin
+u-boot_HS_XIP_X-LOADER: $(obj)/u-boot.bin FORCE
 	$(call if_changed,mkomapsecimg)
 
 # For supporting the SPL loading and interpreting of FIT images whose
@@ -90,21 +91,18 @@ ifdef CONFIG_SPL_LOAD_FIT
 MKIMAGEFLAGS_u-boot_HS.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
 	-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
 	-n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
-	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
+	$(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
 
 OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST)))
 $(OF_LIST_TARGETS): dtbs
 
-%_HS.dtb: %.dtb
+%.dtb_HS: %.dtb FORCE
 	$(call if_changed,omapsecureimg)
-	$(Q)if [ -f $@ ]; then \
-		cp -f $@ $<; \
-	fi
 
-u-boot-nodtb_HS.bin: u-boot-nodtb.bin
+u-boot-nodtb_HS.bin: u-boot-nodtb.bin FORCE
 	$(call if_changed,omapsecureimg)
 
-u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%_HS.dtb,$(OF_LIST_TARGETS))
+u-boot_HS.img: u-boot-nodtb_HS.bin u-boot.img $(patsubst %.dtb,%.dtb_HS,$(OF_LIST_TARGETS)) FORCE
 	$(call if_changed,mkimage)
 	$(Q)if [ -f $@ ]; then \
 		cp -f $@ u-boot.img; \
-- 
2.11.0

[U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation

[Tom Rini]

On Fri, Jan 06, 2017 at 04:20:02PM -0600, Andrew F. Davis wrote:

> When TI_SECURE_DEV_PKG is not defined we warn that the file '*_HS' was
> not generated but generate an unsigned one anyway, first fix this
> warning to say that it was generated but not secured.
> 
> When the user then exports TI_SECURE_DEV_PKG after getting this warning,
> and tries to re-build, 'make' will detect the build artifacts as
> unchanged and so assume they do not need to be re-generated. This causes
> it to fail to sign the files and it will pack unsigned files into the
> final image, even though TI_SECURE_DEV_PKG is now correctly defined and
> working.
> 
> Fix this by using FORCE on the targets causes them to be re-run even if
> the dependent files have not changed.
> 
> This then causes another issue. We currently rename the signed dtb files
> to overwrite the non-signed ones. We do this so the 'mkimage' tool gives
> the packaged dtb sections the correct name. If we do not rename the files
> then SPL will not find them during boot.
> 
> Fix this by renaming the dtb files by appending _HS to the end of the
> filename, after the ".dtb", this causes them to still be named correctly
> in the FIT blob.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170109/dad3ddff/attachment.sig>