From mboxrd@z Thu Jan 1 00:00:00 1970 From: Troy Kisky Date: Mon, 24 Sep 2012 13:54:37 -0700 Subject: [U-Boot] [PATCH V2 02/21] imximage: check dcd_len as entries added In-Reply-To: <505EECE6.6060508@denx.de> References: <1348012989-19674-1-git-send-email-troy.kisky@boundarydevices.com> <1348281558-19520-1-git-send-email-troy.kisky@boundarydevices.com> <1348281558-19520-3-git-send-email-troy.kisky@boundarydevices.com> <505EECE6.6060508@denx.de> Message-ID: <5060C88D.3070507@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> --- >> 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