[U-Boot] [PATCH] x86: zImage: avoid potential NULL dereference

[Heinrich Schuchardt]

On 04/15/2017 06:12 PM, Tom Rini wrote:
> On Sat, Apr 15, 2017 at 03:58:55PM +0200, Heinrich Schuchardt wrote:
> 
>> If bootargs is not assigned getenv("bootargs") will
>> return NULL.
>> Some part of the code is checking for this condition.
>> Other parts dereference a possible NULL pointer.
>>
>> The problem was indicated by cppcheck.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  arch/x86/lib/zimage.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
>> index aafbeb01f9..9b564340a6 100644
>> --- a/arch/x86/lib/zimage.c
>> +++ b/arch/x86/lib/zimage.c
>> @@ -48,12 +48,14 @@ static void build_command_line(char *command_line, int auto_boot)
>>  
>>  	command_line[0] = '\0';
>>  
>> -	env_command_line =  getenv("bootargs");
>> +	env_command_line = getenv("bootargs");
>> +
>> +	if (!env_command_line)
>> +		env_command_line = "";
>>  
>>  	/* set console= argument if we use a serial console */
>>  	if (!strstr(env_command_line, "console=")) {
>>  		if (!strcmp(getenv("stdout"), "serial")) {
>> -
>>  			/* We seem to use serial console */
>>  			sprintf(command_line, "console=ttyS0,%s ",
>>  				getenv("baudrate"));
>> @@ -63,8 +65,7 @@ static void build_command_line(char *command_line, int auto_boot)
>>  	if (auto_boot)
>>  		strcat(command_line, "auto ");
>>  
>> -	if (env_command_line)
>> -		strcat(command_line, env_command_line);
>> +	strcat(command_line, env_command_line);
>>  
>>  	printf("Kernel command line: \"%s\"\n", command_line);
>>  }
> 
> I think this is a false positive from cppcheck.  With env_command_line
> set to NULL, strstr will return NULL.  The only other place we use
> env_command_line is further on where we alrady have a check.  Thanks!
> 

Please, have a look at lib/string.c:
strstr(NULL, b) will happily start searching at 0x0.
So the result will depend on the memory content there.
Should the first bytes be "foo, console=bar\0" the address of "console="
will be returned.
Or maybe a security controller will stop the process due to illegal
memory access.

Best regards

Heinrich Schuchardt

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170415/d30595b2/attachment.sig>