[U-Boot] [RFC/PATCH] common/command.c: Guard strchr/strlen from NULL pointer

Wolfgang Denk wd at denx.de
Wed Nov 24 12:07:19 CET 2010


Dear Thomas Weber,

In message <1290593751-540-1-git-send-email-weber at 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

-- 
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
God grant me the senility to accept the things I cannot  change,  The
frustration  to  try to change things I cannot affect, and the wisdom
to tell the difference.


More information about the U-Boot mailing list