Coverity: gpmc_is_valid_waitpin(): Control flow issues

Coverity: gpmc_is_valid_waitpin(): Control flow issues

[coverity-bot]

Hello!

This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20221104 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:

  Wed Nov 2 10:02:39 2022 -0400
    89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")

Coverity reported the following:

*** CID 1527139:  Control flow issues  (NO_EFFECT)
drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
1042     	spin_unlock(&gpmc_mem_lock);
1043     }
1044     EXPORT_SYMBOL(gpmc_cs_free);
1045
1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
1047     {
vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
1049     }
1050
1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
1052     			      struct gpmc_settings *p)
1053     {

If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):

Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1527139 ("Control flow issues")
Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")

Thanks for your attention!

-- 
Coverity-bot

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Roger Quadros]

Hi Benedikt,

On 04/11/2022 21:33, coverity-bot wrote:
> Hello!
> 
> This is an experimental semi-automated report about issues detected by
> Coverity from a scan of next-20221104 as part of the linux-next scan project:
> https://scan.coverity.com/projects/linux-next-weekly-scan
> 
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by commits:
> 
>   Wed Nov 2 10:02:39 2022 -0400
>     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> 
> Coverity reported the following:
> 
> *** CID 1527139:  Control flow issues  (NO_EFFECT)
> drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
> 1042     	spin_unlock(&gpmc_mem_lock);
> 1043     }
> 1044     EXPORT_SYMBOL(gpmc_cs_free);
> 1045
> 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)

We will need to change this waitpin argument to int.
In addition we will also need to change
struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.

> 1047     {
> vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
> vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
> 1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
> 1049     }
> 1050
> 1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
> 1052     			      struct gpmc_settings *p)
> 1053     {
> 
> If this is a false positive, please let us know so we can mark it as
> such, or teach the Coverity rules to be smarter. If not, please make
> sure fixes get into linux-next. :) For patches fixing this, please
> include these lines (but double-check the "Fixes" first):
> 
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1527139 ("Control flow issues")
> Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> 
> Thanks for your attention!
> 

cheers,
-roger

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Roger Quadros]

On 07/11/2022 10:53, Roger Quadros wrote:
> Hi Benedikt,
> 
> On 04/11/2022 21:33, coverity-bot wrote:
>> Hello!
>>
>> This is an experimental semi-automated report about issues detected by
>> Coverity from a scan of next-20221104 as part of the linux-next scan project:
>> https://scan.coverity.com/projects/linux-next-weekly-scan
>>
>> You're getting this email because you were associated with the identified
>> lines of code (noted below) that were touched by commits:
>>
>>   Wed Nov 2 10:02:39 2022 -0400
>>     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
>>
>> Coverity reported the following:
>>
>> *** CID 1527139:  Control flow issues  (NO_EFFECT)
>> drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
>> 1042     	spin_unlock(&gpmc_mem_lock);
>> 1043     }
>> 1044     EXPORT_SYMBOL(gpmc_cs_free);
>> 1045
>> 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
> 
> We will need to change this waitpin argument to int.
> In addition we will also need to change
> struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
> to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.

Another alternative with less churn is to leave them as u32
but make GPMC_WAITPIN_INVALID set to a large positive number.

> 
>> 1047     {
>> vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
>> vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
>> 1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
>> 1049     }
>> 1050
>> 1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
>> 1052     			      struct gpmc_settings *p)
>> 1053     {
>>
>> If this is a false positive, please let us know so we can mark it as
>> such, or teach the Coverity rules to be smarter. If not, please make
>> sure fixes get into linux-next. :) For patches fixing this, please
>> include these lines (but double-check the "Fixes" first):
>>
>> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
>> Addresses-Coverity-ID: 1527139 ("Control flow issues")
>> Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
>>
>> Thanks for your attention!
>>
> 

cheers,
-roger

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Niedermayr, BENEDIKT]

On Mon, 2022-11-07 at 10:56 +0200, Roger Quadros wrote:
> 
> On 07/11/2022 10:53, Roger Quadros wrote:
> > Hi Benedikt,
> > 
> > On 04/11/2022 21:33, coverity-bot wrote:
> > > Hello!
> > > 
> > > This is an experimental semi-automated report about issues detected by
> > > Coverity from a scan of next-20221104 as part of the linux-next scan project:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&reserved=0
> > > 
> > > You're getting this email because you were associated with the identified
> > > lines of code (noted below) that were touched by commits:
> > > 
> > >   Wed Nov 2 10:02:39 2022 -0400
> > >     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > 
> > > Coverity reported the following:
> > > 
> > > *** CID 1527139:  Control flow issues  (NO_EFFECT)
> > > drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
> > > 1042     	spin_unlock(&gpmc_mem_lock);
> > > 1043     }
> > > 1044     EXPORT_SYMBOL(gpmc_cs_free);
> > > 1045
> > > 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
> > 
> > We will need to change this waitpin argument to int.
> > In addition we will also need to change
> > struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
> > to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.
> 
> Another alternative with less churn is to leave them as u32
> but make GPMC_WAITPIN_INVALID set to a large positive number.
Ok, I will fix that. 
Do I need to send a new fix-patch on top the current patch series? 
Or should I just send only the bugfix-patch for the coverity-bot? 


> 
> > > 1047     {
> > > vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
> > > vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
> > > 1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
> > > 1049     }
> > > 1050
> > > 1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
> > > 1052     			      struct gpmc_settings *p)
> > > 1053     {
> > > 
> > > If this is a false positive, please let us know so we can mark it as
> > > such, or teach the Coverity rules to be smarter. If not, please make
> > > sure fixes get into linux-next. :) For patches fixing this, please
> > > include these lines (but double-check the "Fixes" first):
> > > 
> > > Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> > > Addresses-Coverity-ID: 1527139 ("Control flow issues")
> > > Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > 
> > > Thanks for your attention!
> > > 
> 
> cheers,
> -roger
cheers,
benedikt

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Niedermayr, BENEDIKT]

On Tue, 2022-11-08 at 09:02 +0100, Benedikt Niedermayr wrote:
> On Mon, 2022-11-07 at 10:56 +0200, Roger Quadros wrote:
> > On 07/11/2022 10:53, Roger Quadros wrote:
> > > Hi Benedikt,
> > > 
> > > On 04/11/2022 21:33, coverity-bot wrote:
> > > > Hello!
> > > > 
> > > > This is an experimental semi-automated report about issues detected by
> > > > Coverity from a scan of next-20221104 as part of the linux-next scan project:
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&reserved=0
> > > > 
> > > > You're getting this email because you were associated with the identified
> > > > lines of code (noted below) that were touched by commits:
> > > > 
> > > >   Wed Nov 2 10:02:39 2022 -0400
> > > >     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > > 
> > > > Coverity reported the following:
> > > > 
> > > > *** CID 1527139:  Control flow issues  (NO_EFFECT)
> > > > drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
> > > > 1042     	spin_unlock(&gpmc_mem_lock);
> > > > 1043     }
> > > > 1044     EXPORT_SYMBOL(gpmc_cs_free);
> > > > 1045
> > > > 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
> > > 
> > > We will need to change this waitpin argument to int.
> > > In addition we will also need to change
> > > struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
> > > to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.
> > 
> > Another alternative with less churn is to leave them as u32
> > but make GPMC_WAITPIN_INVALID set to a large positive number.
> Ok, I will fix that. 
> Do I need to send a new fix-patch on top the current patch series? 
> Or should I just send only the bugfix-patch for the coverity-bot? 
> 
Sorry, another Question: 
Is it somehow possible to check locally if the bugfix actually fixed the bug, before I submit the patch?

> 
> > > > 1047     {
> > > > vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
> > > > vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
> > > > 1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
> > > > 1049     }
> > > > 1050
> > > > 1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
> > > > 1052     			      struct gpmc_settings *p)
> > > > 1053     {
> > > > 
> > > > If this is a false positive, please let us know so we can mark it as
> > > > such, or teach the Coverity rules to be smarter. If not, please make
> > > > sure fixes get into linux-next. :) For patches fixing this, please
> > > > include these lines (but double-check the "Fixes" first):
> > > > 
> > > > Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> > > > Addresses-Coverity-ID: 1527139 ("Control flow issues")
> > > > Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > > 
> > > > Thanks for your attention!
> > > > 
> > 
> > cheers,
> > -roger
> cheers,
> benedikt
> 

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Krzysztof Kozlowski]

On 08/11/2022 09:02, Niedermayr, BENEDIKT wrote:
> On Mon, 2022-11-07 at 10:56 +0200, Roger Quadros wrote:
>>
>> On 07/11/2022 10:53, Roger Quadros wrote:
>>> Hi Benedikt,
>>>
>>> On 04/11/2022 21:33, coverity-bot wrote:
>>>> Hello!
>>>>
>>>> This is an experimental semi-automated report about issues detected by
>>>> Coverity from a scan of next-20221104 as part of the linux-next scan project:
>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&reserved=0
>>>>
>>>> You're getting this email because you were associated with the identified
>>>> lines of code (noted below) that were touched by commits:
>>>>
>>>>   Wed Nov 2 10:02:39 2022 -0400
>>>>     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
>>>>
>>>> Coverity reported the following:
>>>>
>>>> *** CID 1527139:  Control flow issues  (NO_EFFECT)
>>>> drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
>>>> 1042     	spin_unlock(&gpmc_mem_lock);
>>>> 1043     }
>>>> 1044     EXPORT_SYMBOL(gpmc_cs_free);
>>>> 1045
>>>> 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
>>>
>>> We will need to change this waitpin argument to int.
>>> In addition we will also need to change
>>> struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
>>> to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.
>>
>> Another alternative with less churn is to leave them as u32
>> but make GPMC_WAITPIN_INVALID set to a large positive number.
> Ok, I will fix that. 
> Do I need to send a new fix-patch on top the current patch series? 
> Or should I just send only the bugfix-patch for the coverity-bot? 
>

A bugfix patch on current next is ok.

Best regards,
Krzysztof

Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Krzysztof Kozlowski]

On 08/11/2022 09:15, Niedermayr, BENEDIKT wrote:

>>> Another alternative with less churn is to leave them as u32
>>> but make GPMC_WAITPIN_INVALID set to a large positive number.
>> Ok, I will fix that. 
>> Do I need to send a new fix-patch on top the current patch series? 
>> Or should I just send only the bugfix-patch for the coverity-bot? 
>>
> Sorry, another Question: 
> Is it somehow possible to check locally if the bugfix actually fixed the bug, before I submit the patch?

I think only if you have Coverity somewhere in your company set for
testing kernel...

Best regards,
Krzysztof