| Message ID | 164359755758.280839.2217137492784656400.stgit@localhost |
|---|---|
| State | Accepted |
| Commit | 53b406369e9d0ba2da1df9b2488976c41acc6332 |
| Headers | show |
| Series | DFU: Update dfu_alt_info parser etc. | expand |
On Mon, Jan 31, 2022 at 11:52:37AM +0900, Masami Hiramatsu wrote: > 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> Applied to u-boot/master, thanks!
Hi Masami, po 31. 1. 2022 v 3:53 odesÃlatel Masami Hiramatsu <masami.hiramatsu@linaro.org> napsal: > > 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> > --- > drivers/dfu/dfu.c | 31 +++++++++++++++++++++------ > drivers/dfu/dfu_mmc.c | 56 ++++++++++++++++++++++++++---------------------- > drivers/dfu/dfu_mtd.c | 36 +++++++++++++++++++------------ > drivers/dfu/dfu_nand.c | 39 ++++++++++++++++++--------------- > drivers/dfu/dfu_ram.c | 25 ++++++++++----------- > drivers/dfu/dfu_sf.c | 38 +++++++++++++++++---------------- > drivers/dfu/dfu_virt.c | 5 +++- > include/dfu.h | 33 ++++++++++++++++++---------- > 8 files changed, 154 insertions(+), 109 deletions(-) > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index 18154774f9..516dda6179 100644 > --- 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 d747ede66c..a91da972d4 100644 > --- 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 27c011f537..c7075f12ec 100644 > --- 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 76761939ab..08e8cf5cdb 100644 > --- 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 361a3ff8af..9d10303164 100644 > --- 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 993e951bc3..25a9c81850 100644 > --- 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 80c99cb06e..29f7a08f67 100644 > --- 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 f6868982df..dcb9cd9d79 100644 > --- 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; > I tried a capsule update on zynq and I expect zynqmp is going to be the same. dfu_alt_info is generated like this by u-boot "mmc 0:1=boot.bin fat 0 1;u-boot.img fat 0 1" when this patch is applied it expects "mmc 0=boot.bin fat 0 1;u-boot.img fat 0 1" I just want to double check it with you because then the following patch needs to be applied to fix current behavior. Thanks, Michal diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index a7d9476326ec..8fa5e023ed0b 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -179,7 +179,7 @@ void set_dfu_alt_info(char *interface, char *devstr) switch ((zynq_slcr_get_boot_mode()) & ZYNQ_BM_MASK) { case ZYNQ_BM_SD: snprintf(buf, DFU_ALT_BUF_LEN, - "mmc 0:1=boot.bin fat 0 1;" + "mmc 0=boot.bin fat 0 1;" "%s fat 0 1", CONFIG_SPL_FS_LOAD_PAYLOAD_NAME); break; case ZYNQ_BM_QSPI: diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 802dfbc0ae7c..daaa581ed4e5 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -662,13 +662,13 @@ void set_dfu_alt_info(char *interface, char *devstr) bootseq = mmc_get_env_dev(); if (!multiboot) snprintf(buf, DFU_ALT_BUF_LEN, - "mmc %d:1=boot.bin fat %d 1;" + "mmc %d=boot.bin fat %d 1;" "%s fat %d 1", bootseq, bootseq, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq); else snprintf(buf, DFU_ALT_BUF_LEN, - "mmc %d:1=boot%04d.bin fat %d 1;" + "mmc %d=boot%04d.bin fat %d 1;" "%s fat %d 1", bootseq, multiboot, bootseq, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq);
On Tue, Aug 09, 2022 at 04:11:40PM +0200, Michal Simek wrote: > Hi Masami, > > po 31. 1. 2022 v 3:53 odesÃlatel Masami Hiramatsu > <masami.hiramatsu@linaro.org> napsal: > > > > 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> > > --- > > drivers/dfu/dfu.c | 31 +++++++++++++++++++++------ > > drivers/dfu/dfu_mmc.c | 56 ++++++++++++++++++++++++++---------------------- > > drivers/dfu/dfu_mtd.c | 36 +++++++++++++++++++------------ > > drivers/dfu/dfu_nand.c | 39 ++++++++++++++++++--------------- > > drivers/dfu/dfu_ram.c | 25 ++++++++++----------- > > drivers/dfu/dfu_sf.c | 38 +++++++++++++++++---------------- > > drivers/dfu/dfu_virt.c | 5 +++- > > include/dfu.h | 33 ++++++++++++++++++---------- > > 8 files changed, 154 insertions(+), 109 deletions(-) > > > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > > index 18154774f9..516dda6179 100644 > > --- 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 d747ede66c..a91da972d4 100644 > > --- 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 27c011f537..c7075f12ec 100644 > > --- 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 76761939ab..08e8cf5cdb 100644 > > --- 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 361a3ff8af..9d10303164 100644 > > --- 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 993e951bc3..25a9c81850 100644 > > --- 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 80c99cb06e..29f7a08f67 100644 > > --- 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 f6868982df..dcb9cd9d79 100644 > > --- 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; > > > > I tried a capsule update on zynq and I expect zynqmp is going to be the same. > dfu_alt_info is generated like this by u-boot > "mmc 0:1=boot.bin fat 0 1;u-boot.img fat 0 1" > > when this patch is applied it expects > "mmc 0=boot.bin fat 0 1;u-boot.img fat 0 1" > > I just want to double check it with you because then the following > patch needs to be applied > to fix current behavior. As you might have noticed, he has left Linaro. Regarding the issue you mentioned above, if you continue to use a partition on your mmc device as firmware storage, there may be a bug in his patch. -Takahiro Akashi > Thanks, > Michal > > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c > index a7d9476326ec..8fa5e023ed0b 100644 > --- a/board/xilinx/zynq/board.c > +++ b/board/xilinx/zynq/board.c > @@ -179,7 +179,7 @@ void set_dfu_alt_info(char *interface, char *devstr) > switch ((zynq_slcr_get_boot_mode()) & ZYNQ_BM_MASK) { > case ZYNQ_BM_SD: > snprintf(buf, DFU_ALT_BUF_LEN, > - "mmc 0:1=boot.bin fat 0 1;" > + "mmc 0=boot.bin fat 0 1;" > "%s fat 0 1", CONFIG_SPL_FS_LOAD_PAYLOAD_NAME); > break; > case ZYNQ_BM_QSPI: > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c > index 802dfbc0ae7c..daaa581ed4e5 100644 > --- a/board/xilinx/zynqmp/zynqmp.c > +++ b/board/xilinx/zynqmp/zynqmp.c > @@ -662,13 +662,13 @@ void set_dfu_alt_info(char *interface, char *devstr) > bootseq = mmc_get_env_dev(); > if (!multiboot) > snprintf(buf, DFU_ALT_BUF_LEN, > - "mmc %d:1=boot.bin fat %d 1;" > + "mmc %d=boot.bin fat %d 1;" > "%s fat %d 1", > bootseq, bootseq, > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq); > else > snprintf(buf, DFU_ALT_BUF_LEN, > - "mmc %d:1=boot%04d.bin fat %d 1;" > + "mmc %d=boot%04d.bin fat %d 1;" > "%s fat %d 1", > bootseq, multiboot, bootseq, > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME, bootseq); > > > > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Xilinx Microblaze > Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs > U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 18154774f9..516dda6179 100644 --- 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 d747ede66c..a91da972d4 100644 --- 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 27c011f537..c7075f12ec 100644 --- 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 76761939ab..08e8cf5cdb 100644 --- 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 361a3ff8af..9d10303164 100644 --- 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 993e951bc3..25a9c81850 100644 --- 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 80c99cb06e..29f7a08f67 100644 --- 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 f6868982df..dcb9cd9d79 100644 --- 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;
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> --- drivers/dfu/dfu.c | 31 +++++++++++++++++++++------ drivers/dfu/dfu_mmc.c | 56 ++++++++++++++++++++++++++---------------------- drivers/dfu/dfu_mtd.c | 36 +++++++++++++++++++------------ drivers/dfu/dfu_nand.c | 39 ++++++++++++++++++--------------- drivers/dfu/dfu_ram.c | 25 ++++++++++----------- drivers/dfu/dfu_sf.c | 38 +++++++++++++++++---------------- drivers/dfu/dfu_virt.c | 5 +++- include/dfu.h | 33 ++++++++++++++++++---------- 8 files changed, 154 insertions(+), 109 deletions(-)