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.g...@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

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to