[PATCH] xfree86: check for NULL pointer before dereferences it in parser code

[Éric Piel]

On 16/04/10 18:17, Alan Coopersmith wrote:
> Tiago Vignatti wrote:
>> Seems to be harmless. Meh.
>>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti at nokia.com>
>> ---
>>  hw/xfree86/parser/scan.c |    9 +++++++--
>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
>> index 8aab0cf..0b9461b 100644
>> --- a/hw/xfree86/parser/scan.c
>> +++ b/hw/xfree86/parser/scan.c
>> @@ -844,11 +844,16 @@ OpenConfigFile(const char *path, const char *cmdline, const char *projroot,
>>  static int
>>  ConfigFilter(const struct dirent *de)
>>  {
>> -	const char *name = de->d_name;
>> +	const char *name;
>>  	size_t len = strlen(name);
>>  	size_t suflen = strlen(XCONFIGSUFFIX);
>>  
>> -	if (!name || name[0] == '.' || len <= suflen)
>> +	if (!name)
>> +		return 0;
>> +
>> +	name = de->d_name;
> 
> NACK.  You are now checking if the uninitialized variable is not-NULL and
> never checking the actual value, as well as calling strlen on the uninitialized
> value.  The problem is that strlen(name) is being called before if (!name), so
> the correct fix would be:
... and you probably want to check de is not NULL as well :-)


Eric
> 
> @@ -845,10 +845,13 @@ static int
>  ConfigFilter(const struct dirent *de)
>  {
>         const char *name = de->d_name;
> -       size_t len = strlen(name);
> +       size_t len;
>         size_t suflen = strlen(XCONFIGSUFFIX);
> 
> -       if (!name || name[0] == '.' || len <= suflen)
> +       if (!name || name[0] == '.')
> +               return 0;
> +       len = strlen(name);
> +       if (len <= suflen)
>                 return 0;
>         if (strcmp(&name[len-suflen], XCONFIGSUFFIX) != 0)
>                 return 0;
> 
> 
>