From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Michal Simek <monstr@monstr.eu>
Cc: Masami Hiramatsu <masami.hiramatsu@linaro.org>,
Tom Rini <trini@konsulko.com>, Lukasz Majewski <lukma@denx.de>,
u-boot@lists.denx.de, ilias.apalodimas@linaro.org,
sughosh.ganu@linaro.org, jaswinder.singh@linaro.org
Subject: Re: [PATCH v2 3/5] DFU: Check the number of arguments and argument string strictly
Date: Wed, 10 Aug 2022 09:19:15 +0900 [thread overview]
Message-ID: <20220810001915.GA7656@laputa> (raw)
In-Reply-To: <CAHTX3dKfeB=ZotpBdVOnEeybsF4Tw4NYT7VtL5V-G1TUPN74eg@mail.gmail.com>
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
next prev parent reply other threads:[~2022-08-10 0:19 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-31 2:52 [PATCH v2 0/5] DFU: Update dfu_alt_info parser etc Masami Hiramatsu
2022-01-31 2:52 ` [PATCH v2 1/5] DFU: Do not copy the entity name over the buffer size Masami Hiramatsu
2022-02-11 17:06 ` Tom Rini
2022-01-31 2:52 ` [PATCH v2 2/5] DFU: Accept redundant spaces and tabs in dfu_alt_info Masami Hiramatsu
2022-02-11 17:06 ` Tom Rini
2022-01-31 2:52 ` [PATCH v2 3/5] DFU: Check the number of arguments and argument string strictly Masami Hiramatsu
2022-02-11 17:06 ` Tom Rini
2022-08-09 14:11 ` Michal Simek
2022-08-10 0:19 ` AKASHI Takahiro [this message]
2022-01-31 2:52 ` [PATCH v2 4/5] doc: usage: DFU: Fix dfu_alt_info document Masami Hiramatsu
2022-02-11 17:06 ` Tom Rini
2022-01-31 2:52 ` [PATCH v2 5/5] cmd/dfu: Enable 'dfu list' command without DFU_OVER_USB Masami Hiramatsu
2022-02-11 17:06 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220810001915.GA7656@laputa \
--to=takahiro.akashi@linaro.org \
--cc=ilias.apalodimas@linaro.org \
--cc=jaswinder.singh@linaro.org \
--cc=lukma@denx.de \
--cc=masami.hiramatsu@linaro.org \
--cc=monstr@monstr.eu \
--cc=sughosh.ganu@linaro.org \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.