common/cmd_mii.c: Add sanity argc check

PATCH New Shinya Kuribayashi 4 participants 7 messages Dec 27, 2007 – Jan 9, 2008

Share & tools

Shinya Kuribayashi

author

2007-12-27 06:39 UTC #0 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

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 <[email protected]>

---

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

common/cmd_mii.c +5 -0

index b771322..b99bd06 100644
@@ -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

Shinya Kuribayashi

author

2007-12-27 06:56 UTC #1 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

Quoted 15 lines · 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 <[email protected]>
---
 common/cmd_mii.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
common/cmd_mii.c +0 -0
index b771322..b99bd06 100644
@@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	int		rcode = 0;

common/cmd_mii.c int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) · line 448

Quoted 10 lines

common/cmd_mii.c +5 -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

gvb.uboot

2007-12-27 13:43 UTC #2 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

Quoted 31 lines · Shinya Kuribayashi

Quoted 15 lines · 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 <[email protected]>
---
 common/cmd_mii.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
common/cmd_mii.c +0 -0
index b771322..b99bd06 100644
@@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	int		rcode = 0;
common/cmd_mii.c int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) · line 448
Quoted 10 lines
common/cmd_mii.c +5 -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.

Quoted 3 lines

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

Andrew Dyer

2007-12-27 15:14 UTC #3 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

In reply to Shinya Kuribayashi

Quoted 16 lines · 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 <[email protected]>
---
 common/cmd_mii.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
common/cmd_mii.c +0 -0
index b771322..b99bd06 100644
@@ -438,6 +438,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
        int             rcode = 0;

common/cmd_mii.c int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) · line 448

Quoted 10 lines

common/cmd_mii.c +5 -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 :-(

Signature · 2 lines

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

Shinya Kuribayashi

author

2007-12-28 01:17 UTC #4 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

Quoted 4 lines · Andrew Dyer

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

Shinya Kuribayashi

author

2007-12-28 03:43 UTC #5 lore raw

PATCH 1/2 cmd_mii.c: Fix a NULL pointer dereference

In reply to 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 <[email protected]>

Signed-off-by Andrew Dyer <[email protected]>

---

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

common/cmd_mii.c +5 -0

index b771322..b99bd06 100644
@@ -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

Wolfgang Denk

Applied

2008-01-09 20:36 UTC #6 lore raw

PATCH 1/1 common/cmd_mii.c: Add sanity argc check

In reply to Shinya Kuribayashi

Quoted 4 lines · In message <[email protected]> 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 <[email protected]>

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 8 lines

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