[U-Boot] [RFC/PATCH] common/command.c: Guard strchr/strlen from NULL pointer
Guard strchr/strlen from being called with NULL pointer. This line is crashing on OMAP3/Devkit8000 when command "env" is called without subcommand.
Toolchain is Codesourcery 2010q1.
The cmd is NULL in this case because the calling function "do_env" decremented the argc without checking if there are still arguments available.
caller: static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ... /* drop initial "env" arg */ argc--; argv++;
cp = find_cmd_tbl(argv[0], cmd_env_sub, ARRAY_SIZE(cmd_env_sub));
Signed-off-by: Thomas Weber weber@corscience.de --- common/command.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c index 0020eac..03a713a 100644 --- a/common/command.c +++ b/common/command.c @@ -105,14 +105,15 @@ cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len) cmd_tbl_t *cmdtp; cmd_tbl_t *cmdtp_temp = table; /*Init value */ const char *p; - int len; + int len = 0; int n_found = 0;
/* * Some commands allow length modifiers (like "cp.b"); * compare command name only until first dot. */ - len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd); + if (cmd != NULL) + len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd);
for (cmdtp = table; cmdtp != table + table_len;
Dear Thomas Weber,
In message 1290593751-540-1-git-send-email-weber@corscience.de you wrote:
Guard strchr/strlen from being called with NULL pointer. This line is crashing on OMAP3/Devkit8000 when command "env" is called without subcommand.
Toolchain is Codesourcery 2010q1.
The cmd is NULL in this case because the calling function "do_env" decremented the argc without checking if there are still arguments available.
One could argue if "env" should be fixed, then.
cmd_tbl_t *cmdtp; cmd_tbl_t *cmdtp_temp = table; /*Init value */ const char *p;
- int len;
int len = 0; int n_found = 0;
/*
- Some commands allow length modifiers (like "cp.b");
- compare command name only until first dot.
*/
- len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd);
- if (cmd != NULL)
len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd);
This is a pretty logish way for a simple thing. It's recommended practice to use a minimal return path for error handling.
Like that:
@@ -108,6 +108,9 @@ cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len) int len; int n_found = 0;
+ if (!cmd) + return NULL; + /* * Some commands allow length modifiers (like "cp.b"); * compare command name only until first dot.
Does this work for you as well? If yes, then please resubmit like this.
Best regards,
Wolfgang Denk
Guard strchr/strlen from being called with NULL pointer. This line is crashing when command "env" is called without subcommand.
The cmd is NULL in this case because the calling function "do_env" decremented the argc without checking if there are still arguments available.
Signed-off-by: Thomas Weber weber@corscience.de --- Changes for v2: - Use shorter way to leave function in error case.
common/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/common/command.c b/common/command.c index 0020eac..0b1a3fb 100644 --- a/common/command.c +++ b/common/command.c @@ -108,6 +108,8 @@ cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len) int len; int n_found = 0;
+ if (!cmd) + return NULL; /* * Some commands allow length modifiers (like "cp.b"); * compare command name only until first dot.
Dear Thomas Weber,
In message 1290600472-23147-1-git-send-email-weber@corscience.de you wrote:
Guard strchr/strlen from being called with NULL pointer. This line is crashing when command "env" is called without subcommand.
The cmd is NULL in this case because the calling function "do_env" decremented the argc without checking if there are still arguments available.
Signed-off-by: Thomas Weber weber@corscience.de
Changes for v2:
- Use shorter way to leave function in error case.
common/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
The env command needs one subcommand. If this is not available print the usage help.
Signed-off-by: Thomas Weber weber@corscience.de --- common/cmd_nvedit.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 3fd8abc..52c5e7c 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -848,6 +848,9 @@ static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *cp;
+ if (argc < 2) + return cmd_usage(cmdtp); + /* drop initial "env" arg */ argc--; argv++;
Dear Thomas Weber,
In message 1290600472-23147-2-git-send-email-weber@corscience.de you wrote:
The env command needs one subcommand. If this is not available print the usage help.
Signed-off-by: Thomas Weber weber@corscience.de
common/cmd_nvedit.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Hello.
On 24-11-2010 13:15, Thomas Weber wrote:
Guard strchr/strlen from being called with NULL pointer. This line is crashing on OMAP3/Devkit8000 when command "env" is called without subcommand.
Toolchain is Codesourcery 2010q1.
The cmd is NULL in this case because the calling function "do_env" decremented the argc without checking if there are still arguments available.
caller: static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) ... /* drop initial "env" arg */ argc--; argv++;
cp = find_cmd_tbl(argv[0], cmd_env_sub, ARRAY_SIZE(cmd_env_sub));
Signed-off-by: Thomas Weberweber@corscience.de
common/command.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/command.c b/common/command.c index 0020eac..03a713a 100644 --- a/common/command.c +++ b/common/command.c @@ -105,14 +105,15 @@ cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len) cmd_tbl_t *cmdtp; cmd_tbl_t *cmdtp_temp = table; /*Init value */ const char *p;
- int len;
int len = 0; int n_found = 0;
/*
- Some commands allow length modifiers (like "cp.b");
- compare command name only until first dot.
*/
- len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd);
- if (cmd != NULL)
len = ((p = strchr(cmd, '.')) == NULL) ? strlen (cmd) : (p - cmd);
checkpatch.pl would complain about the space between 'strlen' and (, so seems a high time to fix this. Besides, it's not consistent with strchr() invocation where there's no space...
WBR, Sergei
participants (3)
-
Sergei Shtylyov -
Thomas Weber -
Wolfgang Denk