[U-Boot-Users] [PATCH] 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 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

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

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

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%...
I thought that fix was in the tree for a long time :-(

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%...
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

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

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
participants (4)
-
Andrew Dyer
-
gvb.uboot
-
Shinya Kuribayashi
-
Wolfgang Denk