[U-Boot] [PATCH 3/3] cbfs: add support for cbfs header components

[Alexander Graf]

On 15.02.18 07:40, Andre Heider wrote:
> This fixes walking the cbfs file list because the bound checks do not
> apply to header components.
> 
> Output of coreboot's cbfstool:
>     Name                           Offset     Type           Size   Comp
>     cbfs master header             0x0        cbfs header        32 none
>     fallback/romstage              0x80       stage           21344 none
>     fallback/ramstage              0x5440     stage           36848 none
>     config                         0xe480     raw               310 none
>     revision                       0xe600     raw               575 none
>     fallback/bl31                  0xe880     payload         15931 none
>     fallback/payload               0x12700    payload        205449 none
>     (empty)                        0x44a00    null           111768 none
>     header pointer                 0x5fec0    cbfs header         4 none
> 
> Output of u-boot's cbfsls:
>          size              type  name
>     ------------------------------------------
>            32       cbfs header  cbfs master header
>         21344             stage  fallback/romstage
>         36848             stage  fallback/ramstage
>           310               raw  config
>           575               raw  revision
>         15931           payload  fallback/bl31
>        205449           payload  fallback/payload
>        111768              null  (empty)
>             4       cbfs header  header pointer

I don't see a before/after comparison? What output exactly did get fixed?

I don't quite understand what case exactly this fixes. The bounds check
seems to try to find out whether the header is within the master header
range, right?

So doesn't this just mean we're reading beyond the expected fs size?


Alex

> 
> Signed-off-by: Andre Heider <a.heider at gmail.com>
> ---
>  cmd/cbfs.c     |  3 +++
>  fs/cbfs/cbfs.c | 10 ++++++----
>  include/cbfs.h |  1 +
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/cbfs.c b/cmd/cbfs.c
> index 736f8c4527..f5ad04c45a 100644
> --- a/cmd/cbfs.c
> +++ b/cmd/cbfs.c
> @@ -113,6 +113,9 @@ static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc,
>  		printf(" %8d", file_cbfs_size(file));
>  
>  		switch (type) {
> +		case CBFS_COMPONENT_CBFSHEADER:
> +			type_name = "cbfs header";
> +			break;
>  		case CBFS_TYPE_STAGE:
>  			type_name = "stage";
>  			break;
> diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
> index 46da8f134f..389f60702b 100644
> --- a/fs/cbfs/cbfs.c
> +++ b/fs/cbfs/cbfs.c
> @@ -97,10 +97,12 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align,
>  		}
>  
>  		swap_file_header(&header, fileHeader);
> -		if (header.offset < sizeof(struct cbfs_fileheader) ||
> -		    header.offset > header.len) {
> -			file_cbfs_result = CBFS_BAD_FILE;
> -			return -1;
> +		if (header.type != CBFS_COMPONENT_CBFSHEADER) {
> +			if (header.offset < sizeof(struct cbfs_fileheader) ||
> +			    header.offset > header.len) {
> +				file_cbfs_result = CBFS_BAD_FILE;
> +				return -1;
> +			}
>  		}
>  		newNode->next = NULL;
>  		newNode->type = header.type;
> diff --git a/include/cbfs.h b/include/cbfs.h
> index f50280107b..d5d9d8ce97 100644
> --- a/include/cbfs.h
> +++ b/include/cbfs.h
> @@ -19,6 +19,7 @@ enum cbfs_result {
>  };
>  
>  enum cbfs_filetype {
> +	CBFS_COMPONENT_CBFSHEADER = 0x02,
>  	CBFS_TYPE_STAGE = 0x10,
>  	CBFS_TYPE_PAYLOAD = 0x20,
>  	CBFS_TYPE_OPTIONROM = 0x30,
>