On Wed, 11 Nov 2015 09:36:37 -0700 Stephen Warren <swar...@wwwdotorg.org> 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
signature.asc
Description: PGP signature