public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] dfu: mmc: call fs functions instead of run_command
Date: Wed, 30 Jan 2019 22:35:38 +0100	[thread overview]
Message-ID: <20190130223538.76f5b9d2@jawa> (raw)
In-Reply-To: <20190125185801.17513-1-simon.k.r.goldschmidt@gmail.com>

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;
>  		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




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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190130/650b69c1/attachment.sig>

  parent reply	other threads:[~2019-01-30 21:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-25 18:58 [U-Boot] [PATCH] dfu: mmc: call fs functions instead of run_command Simon Goldschmidt
2019-01-30 18:44 ` Stephen Warren
2019-01-30 21:38   ` Lukasz Majewski
2019-01-30 21:35 ` Lukasz Majewski [this message]
2019-01-31  2:42 ` [U-Boot] " Tom Rini
2019-01-31 16:35   ` Stephen Warren

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=20190130223538.76f5b9d2@jawa \
    --to=lukma@denx.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox