[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[Shinya Kuribayashi]

If type mii command without arguments, we suffer from uninitialized argv[]
entries; for example we MIPS get stuck by TLB error.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
---

 common/cmd_mii.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mii.c b/common/cmd_mii.c
index b771322..b99bd06 100644
--- a/common/cmd_mii.c
+++ b/common/cmd_mii.c
@@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	int		rcode = 0;
 	char		*devname;
 
+	if (argc < 2) {
+		printf("Usage:\n%s\n", cmdtp->usage);
+		return 1;
+	}
+
 #if defined(CONFIG_8xx) || defined(CONFIG_MCF532x)
 	mii_init ();
 #endif

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[Shinya Kuribayashi]

Shinya Kuribayashi wrote:
> If type mii command without arguments, we suffer from uninitialized argv[]
> entries; for example we MIPS get stuck by TLB error.
> 
> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
> ---
> 
>  common/cmd_mii.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/common/cmd_mii.c b/common/cmd_mii.c
> index b771322..b99bd06 100644
> --- a/common/cmd_mii.c
> +++ b/common/cmd_mii.c
> @@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  	int		rcode = 0;
>  	char		*devname;
>  
> +	if (argc < 2) {
> +		printf("Usage:\n%s\n", cmdtp->usage);
> +		return 1;
> +	}
> +
>  #if defined(CONFIG_8xx) || defined(CONFIG_MCF532x)
>  	mii_init ();
>  #endif

By the way, cmd_mii.c has _another_ do_mii() which is enabled for
CONFIG_TERSE_MII user. But it seems there is no CONFIG_TERSE_MII user.
I just wonder that do_mii() is in the transition to the newer version.

thanks,

  Shinya

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[gvb.uboot]

Shinya Kuribayashi wrote:
> Shinya Kuribayashi wrote:
>> If type mii command without arguments, we suffer from uninitialized argv[]
>> entries; for example we MIPS get stuck by TLB error.
>>
>> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
>> ---
>>
>>  common/cmd_mii.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/common/cmd_mii.c b/common/cmd_mii.c
>> index b771322..b99bd06 100644
>> --- a/common/cmd_mii.c
>> +++ b/common/cmd_mii.c
>> @@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>>  	int		rcode = 0;
>>  	char		*devname;
>>  
>> +	if (argc < 2) {
>> +		printf("Usage:\n%s\n", cmdtp->usage);
>> +		return 1;
>> +	}
>> +
>>  #if defined(CONFIG_8xx) || defined(CONFIG_MCF532x)
>>  	mii_init ();
>>  #endif
> 
> By the way, cmd_mii.c has _another_ do_mii() which is enabled for
> CONFIG_TERSE_MII user. But it seems there is no CONFIG_TERSE_MII user.
> I just wonder that do_mii() is in the transition to the newer version.
> 
> thanks,
> 
>   Shinya

Hi Shinya,

Good find & fix on the argc bug.

When I originally wrote the mii command 6(!) years ago, I wrote a 
verbose version that printed human readable decomposition of the flags, 
etc., and a terse one that didn't print as much stuff and thus had a 
smaller memory footprint.

It sounds like the terse version has withered and died, apparently 
people are only using the verbose version (which is very understandable, 
I do myself).

I propose that you remove what remains of the terse version as part of 
your cleanup patch.

If anybody actually needs the terse version, speak up!

Thanks,
gvb

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[Andrew Dyer]

On Dec 27, 2007 12:39 AM, Shinya Kuribayashi
<shinya.kuribayashi@necel.com> wrote:
> If type mii command without arguments, we suffer from uninitialized argv[]
> entries; for example we MIPS get stuck by TLB error.
>
> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
> ---
>
>  common/cmd_mii.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/common/cmd_mii.c b/common/cmd_mii.c
> index b771322..b99bd06 100644
> --- a/common/cmd_mii.c
> +++ b/common/cmd_mii.c
> @@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>         int             rcode = 0;
>         char            *devname;
>
> +       if (argc < 2) {
> +               printf("Usage:\n%s\n", cmdtp->usage);
> +               return 1;
> +       }
> +
>  #if defined(CONFIG_8xx) || defined(CONFIG_MCF532x)
>         mii_init ();
>  #endif
>

This looks the same as one of the fixes I posted in this thread :
http://news.gmane.org/find-root.php?message_id=%3cc166aa9f0506131006ba8f552%40mail.gmail.com%3e

I thought that fix was in the tree for a long time :-(


-- 
Hardware, n.:
        The parts of a computer system that can be kicked.

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[Shinya Kuribayashi]

Andrew Dyer wrote:
> This looks the same as one of the fixes I posted in this thread :
> http://news.gmane.org/find-root.php?message_id=%3cc166aa9f0506131006ba8f552%40mail.gmail.com%3e
> 
> I thought that fix was in the tree for a long time :-(

Wow, if GMANE holds posted date correctly, it's 2 years and 28 weeks
ago... And to make matters worse Wolfgang took the tears' side of the
patch.

I'll re-submit the patch later including your original description.

  Shinya

[U-Boot-Users] [PATCH 1/2] cmd_mii.c: Fix a NULL pointer dereference

[Shinya Kuribayashi]

Issuing `mii' command with no arguments causes NULL pointer dereference as
parse_line() assignes NULL to argv[1] in such case. Make sure `argc' >= 2
before referencing argv[1], or print the usage message.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
Signed-off-by: Andrew Dyer <amdyer@gmail.com>
---

 common/cmd_mii.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mii.c b/common/cmd_mii.c
index b771322..b99bd06 100644
--- a/common/cmd_mii.c
+++ b/common/cmd_mii.c
@@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	int		rcode = 0;
 	char		*devname;
 
+	if (argc < 2) {
+		printf("Usage:\n%s\n", cmdtp->usage);
+		return 1;
+	}
+
 #if defined(CONFIG_8xx) || defined(CONFIG_MCF532x)
 	mii_init ();
 #endif

[U-Boot-Users] [PATCH] common/cmd_mii.c: Add sanity argc check

[Wolfgang Denk]

In message <477348BA.5090201@necel.com> you wrote:
> If type mii command without arguments, we suffer from uninitialized argv[]
> entries; for example we MIPS get stuck by TLB error.
> 
> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>

Applied, thanks.

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
Many companies that have made themselves dependent on [the  equipment
of  a  certain  major  manufacturer] (and in doing so have sold their
soul to the devil) will collapse under the sheer weight  of  the  un-
mastered complexity of their data processing systems.
          -- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5