u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [U-Boot]  [PATCH 1/1] arm: mach-omap2: Fix secure file generation
@ 2016-12-08 22:48 Andrew F. Davis
  2016-12-08 23:22 ` Tom Rini
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew F. Davis @ 2016-12-08 22:48 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-08 22:48 [U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation Andrew F. Davis
@ 2016-12-08 23:22 ` Tom Rini
  2016-12-09 19:59 ` [U-Boot] [U-Boot, " Tom Rini
  2016-12-12 13:47 ` Tom Rini
  2 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2016-12-08 23:22 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-08 22:48 [U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation Andrew F. Davis
  2016-12-08 23:22 ` Tom Rini
@ 2016-12-09 19:59 ` Tom Rini
  2016-12-09 20:05   ` Andrew F. Davis
  2016-12-12 13:47 ` Tom Rini
  2 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2016-12-09 19:59 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 19:59 ` [U-Boot] [U-Boot, " Tom Rini
@ 2016-12-09 20:05   ` Andrew F. Davis
  2016-12-09 20:10     ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew F. Davis @ 2016-12-09 20:05 UTC (permalink / raw)
  To: u-boot

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?
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 20:05   ` Andrew F. Davis
@ 2016-12-09 20:10     ` Tom Rini
  2016-12-09 20:24       ` Andrew F. Davis
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2016-12-09 20:10 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 20:10     ` Tom Rini
@ 2016-12-09 20:24       ` Andrew F. Davis
  2016-12-09 21:30         ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew F. Davis @ 2016-12-09 20:24 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 20:24       ` Andrew F. Davis
@ 2016-12-09 21:30         ` Tom Rini
  2016-12-09 21:39           ` Andrew F. Davis
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2016-12-09 21:30 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 21:30         ` Tom Rini
@ 2016-12-09 21:39           ` Andrew F. Davis
  2016-12-12 15:30             ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew F. Davis @ 2016-12-09 21:39 UTC (permalink / raw)
  To: u-boot

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-08 22:48 [U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation Andrew F. Davis
  2016-12-08 23:22 ` Tom Rini
  2016-12-09 19:59 ` [U-Boot] [U-Boot, " Tom Rini
@ 2016-12-12 13:47 ` Tom Rini
  2 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2016-12-12 13:47 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [U-Boot, 1/1] arm: mach-omap2: Fix secure file generation
  2016-12-09 21:39           ` Andrew F. Davis
@ 2016-12-12 15:30             ` Tom Rini
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2016-12-12 15:30 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-12-12 15:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-08 22:48 [U-Boot] [PATCH 1/1] arm: mach-omap2: Fix secure file generation Andrew F. Davis
2016-12-08 23:22 ` Tom Rini
2016-12-09 19:59 ` [U-Boot] [U-Boot, " Tom Rini
2016-12-09 20:05   ` Andrew F. Davis
2016-12-09 20:10     ` Tom Rini
2016-12-09 20:24       ` Andrew F. Davis
2016-12-09 21:30         ` Tom Rini
2016-12-09 21:39           ` Andrew F. Davis
2016-12-12 15:30             ` Tom Rini
2016-12-12 13:47 ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).