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

[Troy Kisky <troy.kisky@boundarydevices.com>]

On 9/23/2012 4:05 AM, Stefano Babic wrote:
> 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@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
>

It is a fix, but for a bug that has never happened. So I think it is 
very low priority.
But I can reorder the patches so that this is the 1st in the series, in case
the other patches are never accepted.
I don't think it belongs in the current release.

Troy