The function image_create_config_parse_oneline is pretty complex, and since more parameters will be added to support secure booting, we refactor the function to make it more readable.
Also, when a line contained just a keyword without any parameters, strtok_r returned NULL, which was then indiscriminately fed into atoi, causing a segfault. To correct this, we add a NULL check before feeding the extracted token to atoi, and print an error message in case the token is NULL.
Signed-off-by: Mario Six mario.six@gdsys.cc --- tools/kwbimage.c | 157 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 96 insertions(+), 61 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 7ed5453..1bc0102 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -55,20 +55,39 @@ struct nand_ecc_mode nand_ecc_modes[] = { #define BINARY_MAX_ARGS 8
/* In-memory representation of a line of the configuration file */ + +enum image_cfg_type { + IMAGE_CFG_VERSION = 0x1, + IMAGE_CFG_BOOT_FROM, + IMAGE_CFG_DEST_ADDR, + IMAGE_CFG_EXEC_ADDR, + IMAGE_CFG_NAND_BLKSZ, + IMAGE_CFG_NAND_BADBLK_LOCATION, + IMAGE_CFG_NAND_ECC_MODE, + IMAGE_CFG_NAND_PAGESZ, + IMAGE_CFG_BINARY, + IMAGE_CFG_PAYLOAD, + IMAGE_CFG_DATA, + + IMAGE_CFG_COUNT +} type; + +static const char * const id_strs[] = { + [IMAGE_CFG_VERSION] = "VERSION", + [IMAGE_CFG_BOOT_FROM] = "BOOT_FROM", + [IMAGE_CFG_DEST_ADDR] = "DEST_ADDR", + [IMAGE_CFG_EXEC_ADDR] = "EXEC_ADDR", + [IMAGE_CFG_NAND_BLKSZ] = "NAND_BLKSZ", + [IMAGE_CFG_NAND_BADBLK_LOCATION] = "NAND_BADBLK_LOCATION", + [IMAGE_CFG_NAND_ECC_MODE] = "NAND_ECC_MODE", + [IMAGE_CFG_NAND_PAGESZ] = "NAND_PAGE_SIZE", + [IMAGE_CFG_BINARY] = "BINARY", + [IMAGE_CFG_PAYLOAD] = "PAYLOAD", + [IMAGE_CFG_DATA] = "DATA", +}; + struct image_cfg_element { - enum { - IMAGE_CFG_VERSION = 0x1, - IMAGE_CFG_BOOT_FROM, - IMAGE_CFG_DEST_ADDR, - IMAGE_CFG_EXEC_ADDR, - IMAGE_CFG_NAND_BLKSZ, - IMAGE_CFG_NAND_BADBLK_LOCATION, - IMAGE_CFG_NAND_ECC_MODE, - IMAGE_CFG_NAND_PAGESZ, - IMAGE_CFG_BINARY, - IMAGE_CFG_PAYLOAD, - IMAGE_CFG_DATA, - } type; + enum image_cfg_type type; union { unsigned int version; unsigned int bootfrom; @@ -488,78 +507,94 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params, return image; }
+int recognize_keyword(char *keyword) +{ + int kw_id; + + for (kw_id = 1; kw_id < IMAGE_CFG_COUNT; ++kw_id) + if (!strcmp(keyword, id_strs[kw_id])) + return kw_id; + + return 0; +} + static int image_create_config_parse_oneline(char *line, struct image_cfg_element *el) { - char *keyword, *saveptr; - char deliminiters[] = " \t"; + char *keyword, *saveptr, *value1, *value2; + char delimiters[] = " \t"; + int keyword_id, ret, argi; + char *unknown_msg = "Ignoring unknown line '%s'\n";
- keyword = strtok_r(line, deliminiters, &saveptr); - if (!strcmp(keyword, "VERSION")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); + keyword = strtok_r(line, delimiters, &saveptr); + keyword_id = recognize_keyword(keyword);
- el->type = IMAGE_CFG_VERSION; - el->version = atoi(value); - } else if (!strcmp(keyword, "BOOT_FROM")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - int ret = image_boot_mode_id(value); + if (!keyword_id) { + fprintf(stderr, unknown_msg, line); + return 0; + } + + el->type = keyword_id; + + value1 = strtok_r(NULL, delimiters, &saveptr); + + if (!value1) { + fprintf(stderr, "Parameter missing in line '%s'\n", line); + return -1; + } + + switch (keyword_id) { + case IMAGE_CFG_VERSION: + el->version = atoi(value1); + break; + case IMAGE_CFG_BOOT_FROM: + ret = image_boot_mode_id(value1);
if (ret < 0) { - fprintf(stderr, - "Invalid boot media '%s'\n", value); + fprintf(stderr, "Invalid boot media '%s'\n", value1); return -1; } - el->type = IMAGE_CFG_BOOT_FROM; el->bootfrom = ret; - } else if (!strcmp(keyword, "NAND_BLKSZ")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - - el->type = IMAGE_CFG_NAND_BLKSZ; - el->nandblksz = strtoul(value, NULL, 16); - } else if (!strcmp(keyword, "NAND_BADBLK_LOCATION")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - - el->type = IMAGE_CFG_NAND_BADBLK_LOCATION; - el->nandbadblklocation = - strtoul(value, NULL, 16); - } else if (!strcmp(keyword, "NAND_ECC_MODE")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - int ret = image_nand_ecc_mode_id(value); + break; + case IMAGE_CFG_NAND_BLKSZ: + el->nandblksz = strtoul(value1, NULL, 16); + break; + case IMAGE_CFG_NAND_BADBLK_LOCATION: + el->nandbadblklocation = strtoul(value1, NULL, 16); + break; + case IMAGE_CFG_NAND_ECC_MODE: + ret = image_nand_ecc_mode_id(value1);
if (ret < 0) { - fprintf(stderr, - "Invalid NAND ECC mode '%s'\n", value); + fprintf(stderr, "Invalid NAND ECC mode '%s'\n", value1); return -1; } - el->type = IMAGE_CFG_NAND_ECC_MODE; el->nandeccmode = ret; - } else if (!strcmp(keyword, "NAND_PAGE_SIZE")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - - el->type = IMAGE_CFG_NAND_PAGESZ; - el->nandpagesz = strtoul(value, NULL, 16); - } else if (!strcmp(keyword, "BINARY")) { - char *value = strtok_r(NULL, deliminiters, &saveptr); - int argi = 0; + break; + case IMAGE_CFG_NAND_PAGESZ: + el->nandpagesz = strtoul(value1, NULL, 16); + break; + case IMAGE_CFG_BINARY: + argi = 0;
- el->type = IMAGE_CFG_BINARY; - el->binary.file = strdup(value); + el->binary.file = strdup(value1); while (1) { - value = strtok_r(NULL, deliminiters, &saveptr); + char *value = strtok_r(NULL, delimiters, &saveptr); + if (!value) break; el->binary.args[argi] = strtoul(value, NULL, 16); argi++; if (argi >= BINARY_MAX_ARGS) { fprintf(stderr, - "Too many argument for binary\n"); + "Too many arguments for BINARY\n"); return -1; } } el->binary.nargs = argi; - } else if (!strcmp(keyword, "DATA")) { - char *value1 = strtok_r(NULL, deliminiters, &saveptr); - char *value2 = strtok_r(NULL, deliminiters, &saveptr); + break; + case IMAGE_CFG_DATA: + value2 = strtok_r(NULL, delimiters, &saveptr);
if (!value1 || !value2) { fprintf(stderr, @@ -567,11 +602,11 @@ static int image_create_config_parse_oneline(char *line, return -1; }
- el->type = IMAGE_CFG_DATA; el->regdata.raddr = strtoul(value1, NULL, 16); el->regdata.rdata = strtoul(value2, NULL, 16); - } else { - fprintf(stderr, "Ignoring unknown line '%s'\n", line); + break; + default: + fprintf(stderr, unknown_msg, line); }
return 0;