DFU: Check the number of arguments and argument string strictly

When parsing the dfu_alt_info, check the number of arguments
and argument string strictly. If there is any garbage data
(which is not able to be parsed correctly) in dfu_alt_info,
that means something wrong and user may make a typo or mis-
understanding about the syntax. Since the dfu_alt_info is
used for updating the firmware, this mistake may lead to
brick the hardware.
Thus it should be checked strictly for making sure there
is no mistake.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 1815477..516dda6 100644 (file)
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -500,12 +500,29 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
                           char *interface, char *devstr)
 {
+       char *argv[DFU_MAX_ENTITY_ARGS];
+       int argc;
        char *st;
 
        debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr);
        st = strsep(&s, " \t");
        strlcpy(dfu->name, st, DFU_NAME_SIZE);
-       s = skip_spaces(s);
+
+       /* Parse arguments */
+       for (argc = 0; s && argc < DFU_MAX_ENTITY_ARGS; argc++) {
+               s = skip_spaces(s);
+               if (!*s)
+                       break;
+               argv[argc] = strsep(&s, " \t");
+       }
+
+       if (argc == DFU_MAX_ENTITY_ARGS && s) {
+               s = skip_spaces(s);
+               if (*s) {
+                       log_err("Too many arguments for %s\n", dfu->name);
+                       return -EINVAL;
+               }
+       }
 
        dfu->alt = alt;
        dfu->max_buf_size = 0;
@@ -513,22 +530,22 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
 
        /* Specific for mmc device */
        if (strcmp(interface, "mmc") == 0) {
-               if (dfu_fill_entity_mmc(dfu, devstr, s))
+               if (dfu_fill_entity_mmc(dfu, devstr, argv, argc))
                        return -1;
        } else if (strcmp(interface, "mtd") == 0) {
-               if (dfu_fill_entity_mtd(dfu, devstr, s))
+               if (dfu_fill_entity_mtd(dfu, devstr, argv, argc))
                        return -1;
        } else if (strcmp(interface, "nand") == 0) {
-               if (dfu_fill_entity_nand(dfu, devstr, s))
+               if (dfu_fill_entity_nand(dfu, devstr, argv, argc))
                        return -1;
        } else if (strcmp(interface, "ram") == 0) {
-               if (dfu_fill_entity_ram(dfu, devstr, s))
+               if (dfu_fill_entity_ram(dfu, devstr, argv, argc))
                        return -1;
        } else if (strcmp(interface, "sf") == 0) {
-               if (dfu_fill_entity_sf(dfu, devstr, s))
+               if (dfu_fill_entity_sf(dfu, devstr, argv, argc))
                        return -1;
        } else if (strcmp(interface, "virt") == 0) {
-               if (dfu_fill_entity_virt(dfu, devstr, s))
+               if (dfu_fill_entity_virt(dfu, devstr, argv, argc))
                        return -1;
        } else {
                printf("%s: Device %s not (yet) supported!\n",

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index d747ede..a91da97 100644 (file)
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -337,35 +337,34 @@ void dfu_free_entity_mmc(struct dfu_entity *dfu)
  *     4th (optional):
  *             mmcpart <num> (access to HW eMMC partitions)
  */
-int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
        const char *entity_type;
        ssize_t second_arg;
        size_t third_arg;
-
        struct mmc *mmc;
+       char *s;
 
-       const char *argv[3];
-       const char **parg = argv;
-
-       dfu->data.mmc.dev_num = dectoul(devstr, NULL);
-
-       for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-               *parg = strsep(&s, " \t");
-               if (*parg == NULL) {
-                       pr_err("Invalid number of arguments.\n");
-                       return -ENODEV;
-               }
-               s = skip_spaces(s);
+       if (argc < 3) {
+               pr_err("The number of parameters are not enough.\n");
+               return -EINVAL;
        }
 
+       dfu->data.mmc.dev_num = dectoul(devstr, &s);
+       if (*s)
+               return -EINVAL;
+
        entity_type = argv[0];
        /*
         * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8,
         * with default 10.
         */
-       second_arg = simple_strtol(argv[1], NULL, 0);
-       third_arg = simple_strtoul(argv[2], NULL, 0);
+       second_arg = simple_strtol(argv[1], &s, 0);
+       if (*s)
+               return -EINVAL;
+       third_arg = simple_strtoul(argv[2], &s, 0);
+       if (*s)
+               return -EINVAL;
 
        mmc = find_mmc_device(dfu->data.mmc.dev_num);
        if (mmc == NULL) {
@@ -390,12 +389,14 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
                 * Check for an extra entry at dfu_alt_info env variable
                 * specifying the mmc HW defined partition number
                 */
-               if (s)
-                       if (!strcmp(strsep(&s, " \t"), "mmcpart")) {
-                               s = skip_spaces(s);
-                               dfu->data.mmc.hw_partition =
-                                       simple_strtoul(s, NULL, 0);
+               if (argc > 3) {
+                       if (argc != 5 || strcmp(argv[3], "mmcpart")) {
+                               pr_err("DFU mmc raw accept 'mmcpart <partnum>' option.\n");
+                               return -EINVAL;
                        }
+                       dfu->data.mmc.hw_partition =
+                               simple_strtoul(argv[4], NULL, 0);
+               }
 
        } else if (!strcmp(entity_type, "part")) {
                struct disk_partition partinfo;
@@ -414,15 +415,18 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
                 * Check for an extra entry at dfu_alt_info env variable
                 * specifying the mmc HW defined partition number
                 */
-               if (s)
-                       if (!strcmp(strsep(&s, " \t"), "offset")) {
-                               s = skip_spaces(s);
-                               offset = simple_strtoul(s, NULL, 0);
+               if (argc > 3) {
+                       if (argc != 5 || strcmp(argv[3], "offset")) {
+                               pr_err("DFU mmc raw accept 'mmcpart <partnum>' option.\n");
+                               return -EINVAL;
                        }
+                       dfu->data.mmc.hw_partition =
+                               simple_strtoul(argv[4], NULL, 0);
+               }
 
                dfu->layout                     = DFU_RAW_ADDR;
                dfu->data.mmc.lba_start         = partinfo.start + offset;
-               dfu->data.mmc.lba_size          = partinfo.size-offset;
+               dfu->data.mmc.lba_size          = partinfo.size - offset;
                dfu->data.mmc.lba_blk_size      = partinfo.blksz;
        } else if (!strcmp(entity_type, "fat")) {
                dfu->layout = DFU_FS_FAT;

diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
index 27c011f..c7075f1 100644 (file)
--- a/drivers/dfu/dfu_mtd.c
+++ b/drivers/dfu/dfu_mtd.c
@@ -271,9 +271,9 @@ static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu)
        return DFU_DEFAULT_POLL_TIMEOUT;
 }
 
-int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-       char *st;
+       char *s;
        struct mtd_info *mtd;
        int ret, part;
 
@@ -285,25 +285,33 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
        dfu->dev_type = DFU_DEV_MTD;
        dfu->data.mtd.info = mtd;
        dfu->max_buf_size = mtd->erasesize;
+       if (argc < 1)
+               return -EINVAL;
 
-       st = strsep(&s, " \t");
-       s = skip_spaces(s);
-       if (!strcmp(st, "raw")) {
+       if (!strcmp(argv[0], "raw")) {
+               if (argc != 3)
+                       return -EINVAL;
                dfu->layout = DFU_RAW_ADDR;
-               dfu->data.mtd.start = hextoul(s, &s);
-               if (!isspace(*s))
-                       return -1;
-               s = skip_spaces(s);
-               dfu->data.mtd.size = hextoul(s, &s);
-       } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
+               dfu->data.mtd.start = hextoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
+               dfu->data.mtd.size = hextoul(argv[2], &s);
+               if (*s)
+                       return -EINVAL;
+       } else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) {
                char mtd_id[32];
                struct mtd_device *mtd_dev;
                u8 part_num;
                struct part_info *pi;
 
+               if (argc != 2)
+                       return -EINVAL;
+
                dfu->layout = DFU_RAW_ADDR;
 
-               part = dectoul(s, &s);
+               part = dectoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
 
                sprintf(mtd_id, "%s,%d", devstr, part - 1);
                printf("using id '%s'\n", mtd_id);
@@ -318,10 +326,10 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s)
 
                dfu->data.mtd.start = pi->offset;
                dfu->data.mtd.size = pi->size;
-               if (!strcmp(st, "partubi"))
+               if (!strcmp(argv[0], "partubi"))
                        dfu->data.mtd.ubi = 1;
        } else {
-               printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+               printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
                return -1;
        }
 

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index 7676193..08e8cf5 100644 (file)
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -194,23 +194,25 @@ unsigned int dfu_polltimeout_nand(struct dfu_entity *dfu)
        return DFU_DEFAULT_POLL_TIMEOUT;
 }
 
-int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-       char *st;
+       char *s;
        int ret, dev, part;
 
        dfu->data.nand.ubi = 0;
        dfu->dev_type = DFU_DEV_NAND;
-       st = strsep(&s, " \t");
-       s = skip_spaces(s);
-       if (!strcmp(st, "raw")) {
+       if (argc != 3)
+               return -EINVAL;
+
+       if (!strcmp(argv[0], "raw")) {
                dfu->layout = DFU_RAW_ADDR;
-               dfu->data.nand.start = hextoul(s, &s);
-               if (!isspace(*s))
-                       return -1;
-               s = skip_spaces(s);
-               dfu->data.nand.size = hextoul(s, &s);
-       } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) {
+               dfu->data.nand.start = hextoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
+               dfu->data.nand.size = hextoul(argv[2], &s);
+               if (*s)
+                       return -EINVAL;
+       } else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) {
                char mtd_id[32];
                struct mtd_device *mtd_dev;
                u8 part_num;
@@ -218,11 +220,12 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 
                dfu->layout = DFU_RAW_ADDR;
 
-               dev = dectoul(s, &s);
-               if (!isspace(*s))
-                       return -1;
-               s = skip_spaces(s);
-               part = dectoul(s, &s);
+               dev = dectoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
+               part = dectoul(argv[2], &s);
+               if (*s)
+                       return -EINVAL;
 
                sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
                debug("using id '%s'\n", mtd_id);
@@ -237,10 +240,10 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s)
 
                dfu->data.nand.start = pi->offset;
                dfu->data.nand.size = pi->size;
-               if (!strcmp(st, "partubi"))
+               if (!strcmp(argv[0], "partubi"))
                        dfu->data.nand.ubi = 1;
        } else {
-               printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+               printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
                return -1;
        }
 

diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
index 361a3ff..9d10303 100644 (file)
--- a/drivers/dfu/dfu_ram.c
+++ b/drivers/dfu/dfu_ram.c
@@ -54,18 +54,13 @@ static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset,
        return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset, buf, len);
 }
 
-int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-       const char *argv[3];
-       const char **parg = argv;
-
-       for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
-               *parg = strsep(&s, " \t");
-               if (*parg == NULL) {
-                       pr_err("Invalid number of arguments.\n");
-                       return -ENODEV;
-               }
-               s = skip_spaces(s);
+       char *s;
+
+       if (argc != 3) {
+               pr_err("Invalid number of arguments.\n");
+               return -EINVAL;
        }
 
        dfu->dev_type = DFU_DEV_RAM;
@@ -75,8 +70,12 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s)
        }
 
        dfu->layout = DFU_RAM_ADDR;
-       dfu->data.ram.start = hextoul(argv[1], NULL);
-       dfu->data.ram.size = hextoul(argv[2], NULL);
+       dfu->data.ram.start = hextoul(argv[1], &s);
+       if (*s)
+               return -EINVAL;
+       dfu->data.ram.size = hextoul(argv[2], &s);
+       if (*s)
+               return -EINVAL;
 
        dfu->write_medium = dfu_write_medium_ram;
        dfu->get_medium_size = dfu_get_medium_size_ram;

diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c
index 993e951..25a9c81 100644 (file)
--- a/drivers/dfu/dfu_sf.c
+++ b/drivers/dfu/dfu_sf.c
@@ -166,9 +166,9 @@ static struct spi_flash *parse_dev(char *devstr)
        return dev;
 }
 
-int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
-       char *st;
+       char *s;
        char *devstr_bkup = strdup(devstr);
 
        dfu->data.sf.dev = parse_dev(devstr_bkup);
@@ -179,17 +179,18 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
        dfu->dev_type = DFU_DEV_SF;
        dfu->max_buf_size = dfu->data.sf.dev->sector_size;
 
-       st = strsep(&s, " \t");
-       s = skip_spaces(s);
-       if (!strcmp(st, "raw")) {
+       if (argc != 3)
+               return -EINVAL;
+       if (!strcmp(argv[0], "raw")) {
                dfu->layout = DFU_RAW_ADDR;
-               dfu->data.sf.start = hextoul(s, &s);
-               if (!isspace(*s))
-                       return -1;
-               s = skip_spaces(s);
-               dfu->data.sf.size = hextoul(s, &s);
+               dfu->data.sf.start = hextoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
+               dfu->data.sf.size = hextoul(argv[2], &s);
+               if (*s)
+                       return -EINVAL;
        } else if (CONFIG_IS_ENABLED(DFU_SF_PART) &&
-                  (!strcmp(st, "part") || !strcmp(st, "partubi"))) {
+                  (!strcmp(argv[0], "part") || !strcmp(argv[0], "partubi"))) {
                char mtd_id[32];
                struct mtd_device *mtd_dev;
                u8 part_num;
@@ -198,11 +199,12 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
 
                dfu->layout = DFU_RAW_ADDR;
 
-               dev = dectoul(s, &s);
-               if (!isspace(*s))
-                       return -1;
-               s = skip_spaces(s);
-               part = dectoul(s, &s);
+               dev = dectoul(argv[1], &s);
+               if (*s)
+                       return -EINVAL;
+               part = dectoul(argv[2], &s);
+               if (*s)
+                       return -EINVAL;
 
                sprintf(mtd_id, "%s%d,%d", "nor", dev, part - 1);
                printf("using id '%s'\n", mtd_id);
@@ -216,10 +218,10 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s)
                }
                dfu->data.sf.start = pi->offset;
                dfu->data.sf.size = pi->size;
-               if (!strcmp(st, "partubi"))
+               if (!strcmp(argv[0], "partubi"))
                        dfu->data.sf.ubi = 1;
        } else {
-               printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+               printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]);
                spi_flash_free(dfu->data.sf.dev);
                return -1;
        }

diff --git a/drivers/dfu/dfu_virt.c b/drivers/dfu/dfu_virt.c
index 80c99cb..29f7a08 100644 (file)
--- a/drivers/dfu/dfu_virt.c
+++ b/drivers/dfu/dfu_virt.c
@@ -32,10 +32,13 @@ int __weak dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset,
        return 0;
 }
 
-int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s)
+int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char **argv, int argc)
 {
        debug("%s: devstr = %s\n", __func__, devstr);
 
+       if (argc != 0)
+               return -EINVAL;
+
        dfu->dev_type = DFU_DEV_VIRT;
        dfu->layout = DFU_RAW_ADDR;
        dfu->data.virt.dev_num = dectoul(devstr, NULL);

diff --git a/include/dfu.h b/include/dfu.h
index f686898..dcb9cd9 100644 (file)
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -432,11 +432,15 @@ static inline void dfu_set_defer_flush(struct dfu_entity *dfu)
 int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size);
 
 /* Device specific */
+/* Each entity has 5 arguments in maximum. */
+#define DFU_MAX_ENTITY_ARGS    5
+
 #if CONFIG_IS_ENABLED(DFU_MMC)
-extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
+                              char **argv, int argc);
 #else
 static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
-                                     char *s)
+                                     char **argv, int argc)
 {
        puts("MMC support not available!\n");
        return -1;
@@ -444,10 +448,11 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_NAND)
-extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
+                               char **argv, int argc);
 #else
 static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
-                                      char *s)
+                                      char **argv, int argc)
 {
        puts("NAND support not available!\n");
        return -1;
@@ -455,10 +460,11 @@ static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_RAM)
-extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
+                              char **argv, int argc);
 #else
 static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
-                                     char *s)
+                                     char **argv, int argc)
 {
        puts("RAM support not available!\n");
        return -1;
@@ -466,10 +472,11 @@ static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_SF)
-extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
+                             char **argv, int argc);
 #else
 static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
-                                    char *s)
+                                    char **argv, int argc)
 {
        puts("SF support not available!\n");
        return -1;
@@ -477,10 +484,11 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #if CONFIG_IS_ENABLED(DFU_MTD)
-int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s);
+extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
+                              char **argv, int argc);
 #else
 static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
-                                     char *s)
+                                     char **argv, int argc)
 {
        puts("MTD support not available!\n");
        return -1;
@@ -488,7 +496,8 @@ static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr,
 #endif
 
 #ifdef CONFIG_DFU_VIRT
-int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s);
+int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
+                        char **argv, int argc);
 int dfu_write_medium_virt(struct dfu_entity *dfu, u64 offset,
                          void *buf, long *len);
 int dfu_get_medium_size_virt(struct dfu_entity *dfu, u64 *size);
@@ -496,7 +505,7 @@ int dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset,
                         void *buf, long *len);
 #else
 static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
-                                      char *s)
+                                      char **argv, int argc)
 {
        puts("VIRT support not available!\n");
        return -1;