[U-Boot] [PATCH] dfu: mmc: call fs functions instead of run_command
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com ---
Changes since RFC: - decreased size of 'dev_part_str' to hold 'xxx:yyy' string - removed now unused define 'DFU_CMD_BUF_SIZE'
--- drivers/dfu/dfu_mmc.c | 67 ++++++++++++++++++++----------------------- include/dfu.h | 1 - 2 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index b45e6dc54c..403fd5351d 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -108,17 +108,17 @@ static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu, void *buf, u64 *len) { - const char *fsname, *opname; - char cmd_buf[DFU_CMD_BUF_SIZE]; - char *str_env; + char dev_part_str[8]; int ret; + int fstype; + loff_t size = 0;
switch (dfu->layout) { case DFU_FS_FAT: - fsname = "fat"; + fstype = FS_TYPE_FAT; break; case DFU_FS_EXT4: - fsname = "ext4"; + fstype = FS_TYPE_EXT; break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -126,48 +126,43 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu, return -1; }
+ snprintf(dev_part_str, sizeof(dev_part_str), "%d:%d", + dfu->data.mmc.dev, dfu->data.mmc.part); + + ret = fs_set_blk_dev("mmc", dev_part_str, fstype); + if (ret) { + puts("dfu: fs_set_blk_dev error!\n"); + return ret; + } + switch (op) { case DFU_OP_READ: - opname = "load"; + ret = fs_read(dfu->name, (size_t)buf, 0, 0, &size); + if (ret) { + puts("dfu: fs_read error!\n"); + return ret; + } + *len = size; break; case DFU_OP_WRITE: - opname = "write"; + ret = fs_write(dfu->name, (size_t)buf, 0, *len, &size); + if (ret) { + puts("dfu: fs_write error!\n"); + return ret; + } break; case DFU_OP_SIZE: - opname = "size"; + ret = fs_size(dfu->name, &size); + if (ret) { + puts("dfu: fs_size error!\n"); + return ret; + } + *len = size; break; default: return -1; }
- sprintf(cmd_buf, "%s%s mmc %d:%d", fsname, opname, - dfu->data.mmc.dev, dfu->data.mmc.part); - - if (op != DFU_OP_SIZE) - sprintf(cmd_buf + strlen(cmd_buf), " %p", buf); - - sprintf(cmd_buf + strlen(cmd_buf), " %s", dfu->name); - - if (op == DFU_OP_WRITE) - sprintf(cmd_buf + strlen(cmd_buf), " %llx", *len); - - debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); - - ret = run_command(cmd_buf, 0); - if (ret) { - puts("dfu: Read error!\n"); - return ret; - } - - if (op != DFU_OP_WRITE) { - str_env = env_get("filesize"); - if (str_env == NULL) { - puts("dfu: Wrong file size!\n"); - return -1; - } - *len = simple_strtoul(str_env, NULL, 16); - } - return ret; }
diff --git a/include/dfu.h b/include/dfu.h index fbe978abdc..c671d8b04d 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -80,7 +80,6 @@ struct sf_internal_data { };
#define DFU_NAME_SIZE 32 -#define DFU_CMD_BUF_SIZE 128 #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE #define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8) /* 8 MiB */ #endif
On 1/25/19 11:58 AM, Simon Goldschmidt wrote:
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com
Any chance we could get this applied (directly to u-boot/master) to fix the rampant test failures? Thanks.
Hi Stephen, Tom,
On 1/25/19 11:58 AM, Simon Goldschmidt wrote:
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com
Any chance we could get this applied (directly to u-boot/master) to fix the rampant test failures? Thanks.
Tom, I've acked this patch (in other mail). Please feel free to apply it to -master branch.
Thanks for fixing it :-) (the code was really old and I'm glad that it was updated to fs_* functions).
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi,
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com
Acked-by: Lukasz Majewski lukma@denx.de
Changes since RFC:
- decreased size of 'dev_part_str' to hold 'xxx:yyy' string
- removed now unused define 'DFU_CMD_BUF_SIZE'
drivers/dfu/dfu_mmc.c | 67 ++++++++++++++++++++----------------------- include/dfu.h | 1 - 2 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index b45e6dc54c..403fd5351d 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -108,17 +108,17 @@ static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu, void *buf, u64 *len) {
- const char *fsname, *opname;
- char cmd_buf[DFU_CMD_BUF_SIZE];
- char *str_env;
char dev_part_str[8]; int ret;
int fstype;
loff_t size = 0;
switch (dfu->layout) { case DFU_FS_FAT:
fsname = "fat";
fstype = FS_TYPE_FAT;
fsname = "ext4";
fstype = FS_TYPE_EXT;__func__, @@ -126,48 +126,43 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu, return -1; }
- snprintf(dev_part_str, sizeof(dev_part_str), "%d:%d",
dfu->data.mmc.dev, dfu->data.mmc.part);- ret = fs_set_blk_dev("mmc", dev_part_str, fstype);
- if (ret) {
puts("dfu: fs_set_blk_dev error!\n");return ret;- }
- switch (op) { case DFU_OP_READ:
opname = "load";
ret = fs_read(dfu->name, (size_t)buf, 0, 0, &size);if (ret) {puts("dfu: fs_read error!\n");return ret;}*len = size;
opname = "write";
ret = fs_write(dfu->name, (size_t)buf, 0, *len,&size);
if (ret) {puts("dfu: fs_write error!\n");return ret;}
opname = "size";
ret = fs_size(dfu->name, &size);if (ret) {puts("dfu: fs_size error!\n");return ret;}*len = size;
- sprintf(cmd_buf, "%s%s mmc %d:%d", fsname, opname,
dfu->data.mmc.dev, dfu->data.mmc.part);- if (op != DFU_OP_SIZE)
sprintf(cmd_buf + strlen(cmd_buf), " %p", buf);- sprintf(cmd_buf + strlen(cmd_buf), " %s", dfu->name);
- if (op == DFU_OP_WRITE)
sprintf(cmd_buf + strlen(cmd_buf), " %llx", *len);- debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
- ret = run_command(cmd_buf, 0);
- if (ret) {
puts("dfu: Read error!\n");return ret;- }
- if (op != DFU_OP_WRITE) {
str_env = env_get("filesize");if (str_env == NULL) {puts("dfu: Wrong file size!\n");return -1;}*len = simple_strtoul(str_env, NULL, 16);- }
- return ret;
}
diff --git a/include/dfu.h b/include/dfu.h index fbe978abdc..c671d8b04d 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -80,7 +80,6 @@ struct sf_internal_data { };
#define DFU_NAME_SIZE 32 -#define DFU_CMD_BUF_SIZE 128 #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE #define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8) /* 8 MiB */ #endif
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Fri, Jan 25, 2019 at 07:58:01PM +0100, Simon Goldschmidt wrote:
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
On 1/30/19 7:42 PM, Tom Rini wrote:
On Fri, Jan 25, 2019 at 07:58:01PM +0100, Simon Goldschmidt wrote:
This unbreaks dfu mmc_file_op which is currently broken since using the load cmd on a buffer from heap is not allowed - added with commit aa3c609e2be5 ("fs: prevent overwriting reserved memory")
Fixes: commit aa3c609e2be5 ("fs: prevent overwriting reserved memory") Reported-by: Stephen Warren swarren@wwwdotorg.org Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Tested-by: Stephen Warren swarren@nvidia.com Acked-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
Great, the testing is green again:-) Thanks. (Now we just need -video, -dm, and -net to rebase on u-boot/master so they go green again too).
participants (4)
-
Lukasz Majewski -
Simon Goldschmidt -
Stephen Warren -
Tom Rini