| Message ID | 1563453220-248-3-git-send-email-breno.lima@nxp.com |
|---|---|
| State | Accepted |
| Commit | 40af7d39aab46383e3b0d52b4b06928231807637 |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | Improve HAB support for SPL targets | expand |
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima <breno.lima@nxp.com> wrote: > > Currently it's not possible to authenticate the U-Boot proper of > mx6ul_14x14_evk_defconfig target: > > Authenticate image from DDR location 0x877fffc0... > bad magic magic=0x0 length=0x00 version=0x3 > bad length magic=0x0 length=0x00 version=0x3 > bad version magic=0x0 length=0x00 version=0x3 > spl: ERROR: image authentication fail > > Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and > i.MX7 devices") has increased CSF_SIZE to avoid a possible issue > when booting encrypted boot images. > > Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type > for HAB verification") is hardcoding the CSF and IVT sizes, the > new CSF size is not being considered and u-boot-ivt.img fails to > boot. > > Avoid hardcoded CSF and IVT size to fix this issue. > > Signed-off-by: Breno Lima <breno.lima@nxp.com> Reviewed-by: Fabio Estevam <festevam@gmail.com>
Hi Breno, > Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc. Regards, Peng. > > Currently it's not possible to authenticate the U-Boot proper of > mx6ul_14x14_evk_defconfig target: > > Authenticate image from DDR location 0x877fffc0... > bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 > length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 > spl: ERROR: image authentication fail > > Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and > i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when > booting encrypted boot images. > > Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for > HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not > being considered and u-boot-ivt.img fails to boot. > > Avoid hardcoded CSF and IVT size to fix this issue. > > Signed-off-by: Breno Lima <breno.lima@nxp.com> > --- > common/image.c | 8 +++++--- > tools/default_image.c | 5 ++++- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/common/image.c b/common/image.c index > 9f9538fac2..fc19dfdd9c 100644 > --- a/common/image.c > +++ b/common/image.c > @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong > rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/ > > #include <u-boot/crc.h> > +#include <imximage.h> > +#include <generated/autoconf.h> > > #ifndef CONFIG_SYS_BARGSIZE > #define CONFIG_SYS_BARGSIZE 512 > @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) > } > } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { > printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n", > - image_get_load(hdr) - image_get_header_size(), > - image_get_size(hdr) + image_get_header_size() > - - 0x1FE0); > + image_get_load(hdr) - image_get_header_size(), > + (int)(image_get_size(hdr) + image_get_header_size() > + + sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE)); > } > } > > diff --git a/tools/default_image.c b/tools/default_image.c index > 4b7d1ed4a1..7a26232779 100644 > --- a/tools/default_image.c > +++ b/tools/default_image.c > @@ -19,6 +19,8 @@ > #include <image.h> > #include <tee/optee.h> > #include <u-boot/crc.h> > +#include <imximage.h> > +#include <generated/autoconf.h> > > static image_header_t header; > > @@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat > *sbuf, int ifd, > > if (params->type == IH_TYPE_FIRMWARE_IVT) > /* Add size of CSF minus IVT */ > - imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0; > + imagesize = sbuf->st_size - sizeof(image_header_t) > + + CONFIG_CSF_SIZE - sizeof(flash_header_v2_t); > else > imagesize = sbuf->st_size - sizeof(image_header_t); > > -- > 2.17.1
Hi Peng, Em qua, 11 de set de 2019 às 22:07, Peng Fan <peng.fan@nxp.com> escreveu: > > Hi Breno, > > > Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets > > I saw this patch in imx/master, not in Tom's tree. But this patch breaks > build for other archs, such as arc and etc. > Thanks for reporting the issue, I will submit a fix. Best regards, Breno Lima
Hi Breno, On 12/09/19 03:07, Peng Fan wrote: > Hi Breno, > >> Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets > > I saw this patch in imx/master, not in Tom's tree. But this patch breaks > build for other archs, such as arc and etc. > Any news on this ? I dropped it from u-boot-imx due the breakage, but I can easy pick it up again if it will be fixed. Regards, Stefano > Regards, > Peng. > >> >> Currently it's not possible to authenticate the U-Boot proper of >> mx6ul_14x14_evk_defconfig target: >> >> Authenticate image from DDR location 0x877fffc0... >> bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 >> length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 >> spl: ERROR: image authentication fail >> >> Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and >> i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when >> booting encrypted boot images. >> >> Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for >> HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not >> being considered and u-boot-ivt.img fails to boot. >> >> Avoid hardcoded CSF and IVT size to fix this issue. >> >> Signed-off-by: Breno Lima <breno.lima@nxp.com> >> --- >> common/image.c | 8 +++++--- >> tools/default_image.c | 5 ++++- >> 2 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/common/image.c b/common/image.c index >> 9f9538fac2..fc19dfdd9c 100644 >> --- a/common/image.c >> +++ b/common/image.c >> @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong >> rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/ >> >> #include <u-boot/crc.h> >> +#include <imximage.h> >> +#include <generated/autoconf.h> >> >> #ifndef CONFIG_SYS_BARGSIZE >> #define CONFIG_SYS_BARGSIZE 512 >> @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) >> } >> } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { >> printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n", >> - image_get_load(hdr) - image_get_header_size(), >> - image_get_size(hdr) + image_get_header_size() >> - - 0x1FE0); >> + image_get_load(hdr) - image_get_header_size(), >> + (int)(image_get_size(hdr) + image_get_header_size() >> + + sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE)); >> } >> } >> >> diff --git a/tools/default_image.c b/tools/default_image.c index >> 4b7d1ed4a1..7a26232779 100644 >> --- a/tools/default_image.c >> +++ b/tools/default_image.c >> @@ -19,6 +19,8 @@ >> #include <image.h> >> #include <tee/optee.h> >> #include <u-boot/crc.h> >> +#include <imximage.h> >> +#include <generated/autoconf.h> >> >> static image_header_t header; >> >> @@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat >> *sbuf, int ifd, >> >> if (params->type == IH_TYPE_FIRMWARE_IVT) >> /* Add size of CSF minus IVT */ >> - imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0; >> + imagesize = sbuf->st_size - sizeof(image_header_t) >> + + CONFIG_CSF_SIZE - sizeof(flash_header_v2_t); >> else >> imagesize = sbuf->st_size - sizeof(image_header_t); >> >> -- >> 2.17.1 >
Hi Stefano, Em seg, 16 de set de 2019 às 05:17, Stefano Babic <sbabic@denx.de> escreveu: > > Hi Breno, > > On 12/09/19 03:07, Peng Fan wrote: > > Hi Breno, > > > >> Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets > > > > I saw this patch in imx/master, not in Tom's tree. But this patch breaks > > build for other archs, such as arc and etc. > > > > Any news on this ? I dropped it from u-boot-imx due the breakage, but I > can easy pick it up again if it will be fixed. > Sorry the delay. I'm still trying to reproduce the issue, probably something is missing in my buildman setup. One workaround would be to enclose the code with CONFIG_SECURE_BOOT, this code is only used by IH_TYPE_FIRMWARE_IVT which requires HAB to be enabled. I can send a patch but I would like to confirm before. Thanks, Breno Lima
On 17/09/19 05:26, Breno Matheus Lima wrote: > Hi Stefano, > > Em seg, 16 de set de 2019 às 05:17, Stefano Babic <sbabic@denx.de> escreveu: >> >> Hi Breno, >> >> On 12/09/19 03:07, Peng Fan wrote: >>> Hi Breno, >>> >>>> Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets >>> >>> I saw this patch in imx/master, not in Tom's tree. But this patch breaks >>> build for other archs, such as arc and etc. >>> >> >> Any news on this ? I dropped it from u-boot-imx due the breakage, but I >> can easy pick it up again if it will be fixed. >> > > Sorry the delay. I'm still trying to reproduce the issue, probably > something is missing in my buildman setup. It looks like that a header is missing, maybe this came together with the moving to a common place for gzip & Co (patch for Simon). I think it could be easy for you to reproduce if you set current u-boot-imx. > > One workaround would be to enclose the code with CONFIG_SECURE_BOOT, > this code is only used by IH_TYPE_FIRMWARE_IVT which requires HAB to > be enabled. I can send a patch but I would like to confirm before. Nevertheless, HAB code should be put in just if needed. I agree on the patch independently from the issue. Regards, Stefano > > Thanks, > Breno Lima >
diff --git a/common/image.c b/common/image.c index 9f9538fac2..fc19dfdd9c 100644 --- a/common/image.c +++ b/common/image.c @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/ #include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h> #ifndef CONFIG_SYS_BARGSIZE #define CONFIG_SYS_BARGSIZE 512 @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) } } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n", - image_get_load(hdr) - image_get_header_size(), - image_get_size(hdr) + image_get_header_size() - - 0x1FE0); + image_get_load(hdr) - image_get_header_size(), + (int)(image_get_size(hdr) + image_get_header_size() + + sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE)); } } diff --git a/tools/default_image.c b/tools/default_image.c index 4b7d1ed4a1..7a26232779 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -19,6 +19,8 @@ #include <image.h> #include <tee/optee.h> #include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h> static image_header_t header; @@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd, if (params->type == IH_TYPE_FIRMWARE_IVT) /* Add size of CSF minus IVT */ - imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0; + imagesize = sbuf->st_size - sizeof(image_header_t) + + CONFIG_CSF_SIZE - sizeof(flash_header_v2_t); else imagesize = sbuf->st_size - sizeof(image_header_t);
Currently it's not possible to authenticate the U-Boot proper of mx6ul_14x14_evk_defconfig target: Authenticate image from DDR location 0x877fffc0... bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 spl: ERROR: image authentication fail Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when booting encrypted boot images. Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not being considered and u-boot-ivt.img fails to boot. Avoid hardcoded CSF and IVT size to fix this issue. Signed-off-by: Breno Lima <breno.lima@nxp.com> --- common/image.c | 8 +++++--- tools/default_image.c | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-)