Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Frank Rowand 
On 09/19/17 13:16, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Tue, Sep 19, 2017 at 8:27 PM, Frank Rowand  wrote:
>> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>>> With gcc 4.1.2:
>>>
>>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>>> uninitialized in this function
>>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized 
>>> in this function
>>>
>>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>>> lead to a crash when dereferencing ovinfo later.
>>>
>>> Currently this is a false positive, as the sole creator of of_overlay
>>> structures (of_build_overlay_info(), introduced in commit
>>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>>> this.
>>>
>>> To prevent future issues, add a check for a zero ov->count to
>>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>>> actual compiler warning.
>>>
>>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>>> tree")
>>> Signed-off-by: Geert Uytterhoeven 
>>> ---
>>>  drivers/of/overlay.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>>> --- a/drivers/of/overlay.c
>>> +++ b/drivers/of/overlay.c
>>> @@ -108,7 +108,7 @@ static struct property 
>>> *dup_and_fixup_symbol_prop(struct of_overlay *ov,
>>>   int overlay_name_len;
>>>   int target_path_len;
>>>
>>> - if (!prop->value)
>>> + if (!ov->count || !prop->value)
>>>   return NULL;
>>>   symbol_path = prop->value;
>>>
>>
>> I did not see this patch due to an overzealous spam filter.  I noticed it
>> when Rob replied with his applied email.
>>
>> This check is not needed to prevent accessing overlay_name_len and ovinfo
>> when ov->count == 0.  That is already prevented by:
>>
>> if (k >= ov->count)
>> goto err_free;
>>
>> because k will be zero and ov->count will be zero.
> 
> Thank you, I stand corrected.

No problem.  It's not real obvious, you really need to stop and
ponder.

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Frank Rowand 
On 09/19/17 13:16, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Tue, Sep 19, 2017 at 8:27 PM, Frank Rowand  wrote:
>> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>>> With gcc 4.1.2:
>>>
>>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>>> uninitialized in this function
>>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized 
>>> in this function
>>>
>>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>>> lead to a crash when dereferencing ovinfo later.
>>>
>>> Currently this is a false positive, as the sole creator of of_overlay
>>> structures (of_build_overlay_info(), introduced in commit
>>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>>> this.
>>>
>>> To prevent future issues, add a check for a zero ov->count to
>>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>>> actual compiler warning.
>>>
>>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>>> tree")
>>> Signed-off-by: Geert Uytterhoeven 
>>> ---
>>>  drivers/of/overlay.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>>> --- a/drivers/of/overlay.c
>>> +++ b/drivers/of/overlay.c
>>> @@ -108,7 +108,7 @@ static struct property 
>>> *dup_and_fixup_symbol_prop(struct of_overlay *ov,
>>>   int overlay_name_len;
>>>   int target_path_len;
>>>
>>> - if (!prop->value)
>>> + if (!ov->count || !prop->value)
>>>   return NULL;
>>>   symbol_path = prop->value;
>>>
>>
>> I did not see this patch due to an overzealous spam filter.  I noticed it
>> when Rob replied with his applied email.
>>
>> This check is not needed to prevent accessing overlay_name_len and ovinfo
>> when ov->count == 0.  That is already prevented by:
>>
>> if (k >= ov->count)
>> goto err_free;
>>
>> because k will be zero and ov->count will be zero.
> 
> Thank you, I stand corrected.

No problem.  It's not real obvious, you really need to stop and
ponder.

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Geert Uytterhoeven 
Hi Frank,

On Tue, Sep 19, 2017 at 8:27 PM, Frank Rowand  wrote:
> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>> With gcc 4.1.2:
>>
>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>> uninitialized in this function
>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
>> this function
>>
>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>> lead to a crash when dereferencing ovinfo later.
>>
>> Currently this is a false positive, as the sole creator of of_overlay
>> structures (of_build_overlay_info(), introduced in commit
>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>> this.
>>
>> To prevent future issues, add a check for a zero ov->count to
>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>> actual compiler warning.
>>
>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>> tree")
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  drivers/of/overlay.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
>> of_overlay *ov,
>>   int overlay_name_len;
>>   int target_path_len;
>>
>> - if (!prop->value)
>> + if (!ov->count || !prop->value)
>>   return NULL;
>>   symbol_path = prop->value;
>>
>
> I did not see this patch due to an overzealous spam filter.  I noticed it
> when Rob replied with his applied email.
>
> This check is not needed to prevent accessing overlay_name_len and ovinfo
> when ov->count == 0.  That is already prevented by:
>
> if (k >= ov->count)
> goto err_free;
>
> because k will be zero and ov->count will be zero.

Thank you, I stand corrected.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Geert Uytterhoeven 
Hi Frank,

On Tue, Sep 19, 2017 at 8:27 PM, Frank Rowand  wrote:
> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>> With gcc 4.1.2:
>>
>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>> uninitialized in this function
>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
>> this function
>>
>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>> lead to a crash when dereferencing ovinfo later.
>>
>> Currently this is a false positive, as the sole creator of of_overlay
>> structures (of_build_overlay_info(), introduced in commit
>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>> this.
>>
>> To prevent future issues, add a check for a zero ov->count to
>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>> actual compiler warning.
>>
>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>> tree")
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  drivers/of/overlay.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
>> of_overlay *ov,
>>   int overlay_name_len;
>>   int target_path_len;
>>
>> - if (!prop->value)
>> + if (!ov->count || !prop->value)
>>   return NULL;
>>   symbol_path = prop->value;
>>
>
> I did not see this patch due to an overzealous spam filter.  I noticed it
> when Rob replied with his applied email.
>
> This check is not needed to prevent accessing overlay_name_len and ovinfo
> when ov->count == 0.  That is already prevented by:
>
> if (k >= ov->count)
> goto err_free;
>
> because k will be zero and ov->count will be zero.

Thank you, I stand corrected.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Rob Herring 
On Tue, Sep 19, 2017 at 1:27 PM, Frank Rowand  wrote:
> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>> With gcc 4.1.2:
>>
>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>> uninitialized in this function
>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
>> this function
>>
>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>> lead to a crash when dereferencing ovinfo later.
>>
>> Currently this is a false positive, as the sole creator of of_overlay
>> structures (of_build_overlay_info(), introduced in commit
>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>> this.
>>
>> To prevent future issues, add a check for a zero ov->count to
>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>> actual compiler warning.
>>
>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>> tree")
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  drivers/of/overlay.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
>> of_overlay *ov,
>>   int overlay_name_len;
>>   int target_path_len;
>>
>> - if (!prop->value)
>> + if (!ov->count || !prop->value)
>>   return NULL;
>>   symbol_path = prop->value;
>>
>>
>
> I did not see this patch due to an overzealous spam filter.  I noticed it
> when Rob replied with his applied email.
>
> This check is not needed to prevent accessing overlay_name_len and ovinfo
> when ov->count == 0.  That is already prevented by:
>
> if (k >= ov->count)
> goto err_free;
>
> because k will be zero and ov->count will be zero.

Okay, will drop.

Rob

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Rob Herring 
On Tue, Sep 19, 2017 at 1:27 PM, Frank Rowand  wrote:
> On 09/10/17 03:26, Geert Uytterhoeven wrote:
>> With gcc 4.1.2:
>>
>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
>> uninitialized in this function
>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
>> this function
>>
>> Indeed, if ov->count == 0, both variables are uninitialized, which may
>> lead to a crash when dereferencing ovinfo later.
>>
>> Currently this is a false positive, as the sole creator of of_overlay
>> structures (of_build_overlay_info(), introduced in commit
>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
>> this.
>>
>> To prevent future issues, add a check for a zero ov->count to
>> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
>> actual compiler warning.
>>
>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
>> tree")
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>>  drivers/of/overlay.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
>> of_overlay *ov,
>>   int overlay_name_len;
>>   int target_path_len;
>>
>> - if (!prop->value)
>> + if (!ov->count || !prop->value)
>>   return NULL;
>>   symbol_path = prop->value;
>>
>>
>
> I did not see this patch due to an overzealous spam filter.  I noticed it
> when Rob replied with his applied email.
>
> This check is not needed to prevent accessing overlay_name_len and ovinfo
> when ov->count == 0.  That is already prevented by:
>
> if (k >= ov->count)
> goto err_free;
>
> because k will be zero and ov->count will be zero.

Okay, will drop.

Rob

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Frank Rowand 
On 09/10/17 03:26, Geert Uytterhoeven wrote:
> With gcc 4.1.2:
> 
> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
> uninitialized in this function
> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
> this function
> 
> Indeed, if ov->count == 0, both variables are uninitialized, which may
> lead to a crash when dereferencing ovinfo later.
> 
> Currently this is a false positive, as the sole creator of of_overlay
> structures (of_build_overlay_info(), introduced in commit
> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
> this.
> 
> To prevent future issues, add a check for a zero ov->count to
> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
> actual compiler warning.
> 
> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
> tree")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/of/overlay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
> of_overlay *ov,
>   int overlay_name_len;
>   int target_path_len;
>  
> - if (!prop->value)
> + if (!ov->count || !prop->value)
>   return NULL;
>   symbol_path = prop->value;
>  
> 

I did not see this patch due to an overzealous spam filter.  I noticed it
when Rob replied with his applied email.

This check is not needed to prevent accessing overlay_name_len and ovinfo
when ov->count == 0.  That is already prevented by:

if (k >= ov->count)
goto err_free;

because k will be zero and ov->count will be zero.

-Frank

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Frank Rowand 
On 09/10/17 03:26, Geert Uytterhoeven wrote:
> With gcc 4.1.2:
> 
> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
> uninitialized in this function
> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
> this function
> 
> Indeed, if ov->count == 0, both variables are uninitialized, which may
> lead to a crash when dereferencing ovinfo later.
> 
> Currently this is a false positive, as the sole creator of of_overlay
> structures (of_build_overlay_info(), introduced in commit
> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
> this.
> 
> To prevent future issues, add a check for a zero ov->count to
> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
> actual compiler warning.
> 
> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
> tree")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/of/overlay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct 
> of_overlay *ov,
>   int overlay_name_len;
>   int target_path_len;
>  
> - if (!prop->value)
> + if (!ov->count || !prop->value)
>   return NULL;
>   symbol_path = prop->value;
>  
> 

I did not see this patch due to an overzealous spam filter.  I noticed it
when Rob replied with his applied email.

This check is not needed to prevent accessing overlay_name_len and ovinfo
when ov->count == 0.  That is already prevented by:

if (k >= ov->count)
goto err_free;

because k will be zero and ov->count will be zero.

-Frank

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Rob Herring 
On Sun, Sep 10, 2017 at 12:26:04PM +0200, Geert Uytterhoeven wrote:
> With gcc 4.1.2:
> 
> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
> uninitialized in this function
> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
> this function
> 
> Indeed, if ov->count == 0, both variables are uninitialized, which may
> lead to a crash when dereferencing ovinfo later.
> 
> Currently this is a false positive, as the sole creator of of_overlay
> structures (of_build_overlay_info(), introduced in commit
> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
> this.
> 
> To prevent future issues, add a check for a zero ov->count to
> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
> actual compiler warning.
> 
> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
> tree")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/of/overlay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied.

Rob

Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop()

2017-09-19 Thread Rob Herring 
On Sun, Sep 10, 2017 at 12:26:04PM +0200, Geert Uytterhoeven wrote:
> With gcc 4.1.2:
> 
> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’:
> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used 
> uninitialized in this function
> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in 
> this function
> 
> Indeed, if ov->count == 0, both variables are uninitialized, which may
> lead to a crash when dereferencing ovinfo later.
> 
> Currently this is a false positive, as the sole creator of of_overlay
> structures (of_build_overlay_info(), introduced in commit
> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for
> this.
> 
> To prevent future issues, add a check for a zero ov->count to
> dup_and_fixup_symbol_prop().  Note that this does not get rid of the
> actual compiler warning.
> 
> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device 
> tree")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/of/overlay.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied.

Rob