Re: [cbootimage PATCH 2/3] Fix image update with image smaller than 10KiB

[Alban Bedel <alban.bedel-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

On Mon, 7 Dec 2015 10:20:56 -0700
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:

> On 12/07/2015 04:51 AM, Alban Bedel wrote:
> > On Wed, 11 Nov 2015 09:36:37 -0700
> > Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.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.
> 
> Why? This data that's read is solely used to determine the size of the 
> BCT, and then thrown away. I don't believe any of the data between 
> SIZE_MIN and SIZE_MAX should ever be used; observe the following at the 
> end of the function:
> 
>          context->bct = buffer;
>          if (data_is_valid_bct(context) && g_soc_config->get_bct_size)
>                  bct_size = g_soc_config->get_bct_size();
> 
>          fclose(fp);
>          context->bct = 0; <<<<<<<<

Right, I overlooked that, v3 is under way.

Alban


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]