[U-Boot] [PATCH V2 02/21] imximage: check dcd_len as entries added

[Stefano Babic]

On 22/09/2012 04:38, Troy Kisky wrote:
> Before the len was checked after the entire file
> was processed, so it could have already overflowed.
> 

Hi Troy,

> Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
> ---
>  tools/imximage.c |   26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/imximage.c b/tools/imximage.c
> index 25d3b74..0bfbec3 100644
> --- a/tools/imximage.c
> +++ b/tools/imximage.c
> @@ -71,6 +71,7 @@ static set_dcd_val_t set_dcd_val;
>  static set_dcd_rst_t set_dcd_rst;
>  static set_imx_hdr_t set_imx_hdr;
>  static set_imx_size_t set_imx_size;
> +static uint32_t max_dcd_entries;
>  static uint32_t g_flash_offset;
>  
>  static struct image_type_params imximage_params;
> @@ -173,13 +174,6 @@ static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len,
>  {
>  	dcd_v1_t *dcd_v1 = &imxhdr->header.hdr_v1.dcd_table;
>  
> -	if (dcd_len > MAX_HW_CFG_SIZE_V1) {
> -		fprintf(stderr, "Error: %s[%d] -"
> -			"DCD table exceeds maximum size(%d)\n",
> -			name, lineno, MAX_HW_CFG_SIZE_V1);
> -		exit(EXIT_FAILURE);
> -	}
> -
>  	dcd_v1->preamble.barker = DCD_BARKER;
>  	dcd_v1->preamble.length = dcd_len * sizeof(dcd_type_addr_data_t);
>  }
> @@ -193,13 +187,6 @@ static void set_dcd_rst_v2(struct imx_header *imxhdr, uint32_t dcd_len,
>  {
>  	dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
>  
> -	if (dcd_len > MAX_HW_CFG_SIZE_V2) {
> -		fprintf(stderr, "Error: %s[%d] -"
> -			"DCD table exceeds maximum size(%d)\n",
> -			name, lineno, MAX_HW_CFG_SIZE_V2);
> -		exit(EXIT_FAILURE);
> -	}
> -
>  	dcd_v2->header.tag = DCD_HEADER_TAG;
>  	dcd_v2->header.length = cpu_to_be16(
>  			dcd_len * sizeof(dcd_addr_data_t) + 8);
> @@ -293,12 +280,14 @@ static void set_hdr_func(struct imx_header *imxhdr)
>  		set_dcd_rst = set_dcd_rst_v1;
>  		set_imx_hdr = set_imx_hdr_v1;
>  		set_imx_size = set_imx_size_v1;
> +		max_dcd_entries = MAX_HW_CFG_SIZE_V1;
>  		break;
>  	case IMXIMAGE_V2:
>  		set_dcd_val = set_dcd_val_v2;
>  		set_dcd_rst = set_dcd_rst_v2;
>  		set_imx_hdr = set_imx_hdr_v2;
>  		set_imx_size = set_imx_size_v2;
> +		max_dcd_entries = MAX_HW_CFG_SIZE_V2;
>  		break;
>  	default:
>  		err_imximage_version(imximage_version);
> @@ -425,8 +414,15 @@ static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd,
>  		value = get_cfg_value(token, name, lineno);
>  		(*set_dcd_val)(imxhdr, name, lineno, fld, value, *dcd_len);
>  
> -		if (fld == CFG_REG_VALUE)
> +		if (fld == CFG_REG_VALUE) {
>  			(*dcd_len)++;
> +			if (*dcd_len > max_dcd_entries) {
> +				fprintf(stderr, "Error: %s[%d] -"
> +					"DCD table exceeds maximum size(%d)\n",
> +					name, lineno, max_dcd_entries);
> +				exit(EXIT_FAILURE);
> +			}
> +		}
>  		break;
>  	default:
>  		break;
> 

This patch seems to me unrelated to the rest, and fixes the case when
too much DCD entries are put into the imximage.cfg file. What about to
rebase it on the current code and post it as separate patch ? I think
this can be merged directly, also in the current realease.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================