[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[Lei Wen]

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 drivers/mmc/mmc.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index cf4ea16..5ff1c54 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -92,6 +92,11 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
 
 	blklen = mmc->write_bl_len;
 
+	if (start > mmc->block_dev.lba || (start + blkcnt) > mmc->block_dev.lba) {
+		puts("operation exceed mmc boundary..\n
+		     This devices only have 0x%x blocks\n", mmc->block_dev.lba);
+		return 0;
+	}
 	err = mmc_set_blocklen(mmc, mmc->write_bl_len);
 
 	if (err) {
@@ -219,6 +224,11 @@ static ulong mmc_bread(int dev_num, ulong start, lbaint_t blkcnt, void *dst)
 	if (!mmc)
 		return 0;
 
+	if (start > mmc->block_dev.lba || (start + blkcnt) > mmc->block_dev.lba) {
+		puts("operation exceed mmc boundary..\n
+		     This devices only have 0x%x blocks\n", mmc->block_dev.lba);
+		return 0;
+	}
 	/* We always do full block reads from the card */
 	err = mmc_set_blocklen(mmc, mmc->read_bl_len);
 
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] mmc: print out avaible partition table

[Lei Wen]

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 common/cmd_mmc.c |   20 ++++++++++++++++++++
 disk/part.c      |    3 +++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index c0b30d8..fe04f8d 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -154,6 +154,25 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			mmc_init(mmc);
 
 			return 0;
+		} else if (strncmp(argv[1], "part", 4) == 0) {
+			int dev = simple_strtoul(argv[2], NULL, 10);
+			block_dev_desc_t *mmc_dev;
+			struct mmc *mmc = find_mmc_device(dev);
+
+			if (!mmc) {
+				puts("no mmc devices available\n");
+				return 1;
+			}
+			mmc_init(mmc);
+			mmc_dev = mmc_get_dev(dev);
+			if (mmc_dev != NULL &&
+					mmc_dev->type != DEV_TYPE_UNKNOWN) {
+				print_part(mmc_dev);
+				return 0;
+			}
+
+			puts("get mmc type error!\n");
+			return 1;
 		}
 
 	case 0:
@@ -230,5 +249,6 @@ U_BOOT_CMD(
 	"read <device num> addr blk# cnt\n"
 	"mmc write <device num> addr blk# cnt\n"
 	"mmc rescan <device num>\n"
+	"mmc part <device num>- lists avaiable partition on mmc\n"
 	"mmc list - lists available devices");
 #endif
diff --git a/disk/part.c b/disk/part.c
index 3ba88c7..1806fe6 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -364,6 +364,9 @@ static void print_part_header (const char *type, block_dev_desc_t * dev_desc)
 	case IF_TYPE_DOC:
 		puts ("DOC");
 		break;
+	case IF_TYPE_MMC:
+		puts ("MMC");
+		break;
 	default:
 		puts ("UNKNOWN");
 		break;
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[Sergei Shtylyov]

Hello.

On 13-09-2010 8:06, Lei Wen wrote:

> Signed-off-by: Lei Wen<leiwen@marvell.com>
> ---
>   drivers/mmc/mmc.c |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)

> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index cf4ea16..5ff1c54 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -92,6 +92,11 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
>
>   	blklen = mmc->write_bl_len;
>
> +	if (start > mmc->block_dev.lba || (start + blkcnt) > mmc->block_dev.lba) {

    The second comparison should suffice, no? Also, parens are not necessary.

> +		puts("operation exceed mmc boundary..\n
 > +		     This devices only have 0x%x blocks\n",
 > mmc->block_dev.lba);

    Where's the cloging quote? String literals are not broken up like this -- 
this adds extra spaces to them. Should be:

		puts("operation exceed mmc boundary..\n"
		     "This devices only have 0x%x blocks\n", mmc->block_dev.lba);

> +		return 0;
> +	}
>   	err = mmc_set_blocklen(mmc, mmc->write_bl_len);
>
>   	if (err) {
> @@ -219,6 +224,11 @@ static ulong mmc_bread(int dev_num, ulong start, lbaint_t blkcnt, void *dst)
>   	if (!mmc)
>   		return 0;
>
> +	if (start > mmc->block_dev.lba || (start + blkcnt) > mmc->block_dev.lba) {
> +		puts("operation exceed mmc boundary..\n
> +		     This devices only have 0x%x blocks\n", mmc->block_dev.lba);

    Same comments here...

WBR, Sergei

[U-Boot] [PATCH 2/2] mmc: print out avaible partition table

[Sergei Shtylyov]

Hello.

On 13-09-2010 8:06, Lei Wen wrote:

> Signed-off-by: Lei Wen<leiwen@marvell.com>
> ---
>   common/cmd_mmc.c |   20 ++++++++++++++++++++
>   disk/part.c      |    3 +++
>   2 files changed, 23 insertions(+), 0 deletions(-)

> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
> index c0b30d8..fe04f8d 100644
> --- a/common/cmd_mmc.c
> +++ b/common/cmd_mmc.c
> @@ -154,6 +154,25 @@ int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   			mmc_init(mmc);
>
>   			return 0;
> +		} else if (strncmp(argv[1], "part", 4) == 0) {
> +			int dev = simple_strtoul(argv[2], NULL, 10);
> +			block_dev_desc_t *mmc_dev;
> +			struct mmc *mmc = find_mmc_device(dev);
> +
> +			if (!mmc) {
> +				puts("no mmc devices available\n");
> +				return 1;
> +			}
> +			mmc_init(mmc);
> +			mmc_dev = mmc_get_dev(dev);
> +			if (mmc_dev != NULL &&
> +					mmc_dev->type != DEV_TYPE_UNKNOWN) {

    Too much indentation.

WBR, Sergei

[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[Lei Wen]

On Mon, Sep 13, 2010 at 4:52 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 13-09-2010 8:06, Lei Wen wrote:
>
>> Signed-off-by: Lei Wen<leiwen@marvell.com>
>> ---
>> ?drivers/mmc/mmc.c | ? 10 ++++++++++
>> ?1 files changed, 10 insertions(+), 0 deletions(-)
>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index cf4ea16..5ff1c54 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -92,6 +92,11 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt,
>> const void*src)
>>
>> ? ? ? ?blklen = mmc->write_bl_len;
>>
>> + ? ? ? if (start > mmc->block_dev.lba || (start + blkcnt) >
>> mmc->block_dev.lba) {
>
> ? The second comparison should suffice, no? Also, parens are not necessary.
Em, that is correct.
>
>> + ? ? ? ? ? ? ? puts("operation exceed mmc boundary..\n
>
>> + ? ? ? ? ? ? ? ? ?This devices only have 0x%x blocks\n",
>> mmc->block_dev.lba);
>
> ? Where's the cloging quote? String literals are not broken up like this --
> this adds extra spaces to them. Should be:

I see...
Thanks,
Lei

>
> ? ? ? ? ? ? ? ?puts("operation exceed mmc boundary..\n"
> ? ? ? ? ? ? ? ? ? ? "This devices only have 0x%x blocks\n",
> mmc->block_dev.lba);
>
>> + ? ? ? ? ? ? ? return 0;
>> + ? ? ? }
>> ? ? ? ?err = mmc_set_blocklen(mmc, mmc->write_bl_len);
>>
>> ? ? ? ?if (err) {
>> @@ -219,6 +224,11 @@ static ulong mmc_bread(int dev_num, ulong start,
>> lbaint_t blkcnt, void *dst)
>> ? ? ? ?if (!mmc)
>> ? ? ? ? ? ? ? ?return 0;
>>
>> + ? ? ? if (start > mmc->block_dev.lba || (start + blkcnt) >
>> mmc->block_dev.lba) {
>> + ? ? ? ? ? ? ? puts("operation exceed mmc boundary..\n
>> + ? ? ? ? ? ? ? ? ? ?This devices only have 0x%x blocks\n",
>> mmc->block_dev.lba);
>
> ? Same comments here...
>
> WBR, Sergei
>

[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[Lei Wen]

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 drivers/mmc/mmc.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index cf4ea16..42638f6 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -92,6 +92,11 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
 
 	blklen = mmc->write_bl_len;
 
+	if ((start + blkcnt) > mmc->block_dev.lba) {
+		puts("operation exceed mmc boundary..\n"
+		     "This devices only have 0x%x blocks\n", mmc->block_dev.lba);
+		return 0;
+	}
 	err = mmc_set_blocklen(mmc, mmc->write_bl_len);
 
 	if (err) {
@@ -219,6 +224,11 @@ static ulong mmc_bread(int dev_num, ulong start, lbaint_t blkcnt, void *dst)
 	if (!mmc)
 		return 0;
 
+	if ((start + blkcnt) > mmc->block_dev.lba) {
+		puts("operation exceed mmc boundary..\n"
+		     "This devices only have 0x%x blocks\n", mmc->block_dev.lba);
+		return 0;
+	}
 	/* We always do full block reads from the card */
 	err = mmc_set_blocklen(mmc, mmc->read_bl_len);
 
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] mmc: add boundary check for mmc operation

[Wolfgang Denk]

Dear Lei Wen,

when posting new versions of patches, please always

1) make sure the messages are properly threaded, i. e. make sure to
   provide proper references to the previous postings;
   "git send-email" asks for the message ID.
2) show in the Subject that tthios is a new version of the patch (for
   example by using "[PATCH 1/2 v2]", and add (in the comment section,
   i. e. below the "---" line, a description of what was changed
   respective to the previous version(s).

Thanks.

In message <1284369459-29997-1-git-send-email-leiwen@marvell.com> you wrote:
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> ---
>  drivers/mmc/mmc.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index cf4ea16..42638f6 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -92,6 +92,11 @@ mmc_bwrite(int dev_num, ulong start, lbaint_t blkcnt, const void*src)
>  
>  	blklen = mmc->write_bl_len;
>  
> +	if ((start + blkcnt) > mmc->block_dev.lba) {
> +		puts("operation exceed mmc boundary..\n"
> +		     "This devices only have 0x%x blocks\n", mmc->block_dev.lba);

puts() does not perform output formatting. You want printf() here (and
below).

Also, try to use a shorter message, for example:

	printf("MMC: block number 0x%x = %d exceeds max (0x%x = %d)", ...);


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Beauty is transitory." "Beauty survives."
	-- Spock and Kirk, "That Which Survives", stardate unknown