On Wed, 11 Nov 2015 09:36:37 -0700
Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
> On 11/05/2015 09:03 AM, Alban Bedel wrote:
> > The BCT size check assume a quiet large image, however if the image
> > doesn't contains a bootloader it won't be that large. Change the size
> > check to check for the smallest possible BCT size which is currently
>
> > diff --git a/src/cbootimage.h b/src/cbootimage.h
>
> > +#define NVBOOT_CONFIG_TABLE_SIZE_MIN 4080
>
> I think a comment is warranted here. This value needs to be (a) small
> enough that it isn't larger than the total BCT size on any chip, and (b)
> large enough that it includes the bct->boot_data_version field for all
> chips. (Hopefully those two constraints can continue to be met with a
> single value in the future...)
I'll add this in the next patch.
> > diff --git a/src/data_layout.c b/src/data_layout.c
>
> > @@ -1052,7 +1052,7 @@ int get_bct_size_from_image(build_image_context *context)
> > if (!fp)
> > return -ENODATA;
> >
> > - if (fread(buffer, 1, NVBOOT_CONFIG_TABLE_SIZE_MAX, fp) != NVBOOT_CONFIG_TABLE_SIZE_MAX) {
> > + if (fread(buffer, 1, NVBOOT_CONFIG_TABLE_SIZE_MAX, fp) < NVBOOT_CONFIG_TABLE_SIZE_MIN) {
>
> Can you please also update the size of buffer[]:
No, we still need to read up to NVBOOT_CONFIG_TABLE_SIZE_MAX if needed.
> int get_bct_size_from_image(build_image_context *context)
> {
> u_int8_t buffer[NVBOOT_CONFIG_TABLE_SIZE_MAX];
>
> I wonder if it's worth updating all SoCs' versions of
> if_bct_is_tNNN_get_soc_config() so that they validate that the end
> offset of bct->boot_data_version is < NVBOOT_CONFIG_TABLE_SIZE_MIN, or
> perhaps that the offset is < context->bct_size, in which case
> get_bct_size_from_image() would need to be enhanced to set/clear that
> value when setting/clearing context->bct?
It would be better but it is currently not really needed. Currently the
furthest boot_data_version field is on T124/T132 at an offset of 1744
bytes. Still quiet far from the 4080 bytes that are needed for the
smallest BCT.
Alban
Attachment:
signature.asc
Description: PGP signature
