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

Troy Kisky
2012-09-24T20:54:37Z

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