Skip to contentSkip to conversation

Coverity: gpmc_is_valid_waitpin(): Control flow issues

DISCUSSION coverity-bot 4 participants 7 messages Nov 4, 2022 – Nov 8, 2022 Linux-next tree

lore
Share & tools
lore
#0 lore raw

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):

Addresses-Coverity-ID: 1527139 ("Control flow issues")

Fixes 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")

Thanks for your attention!

Signature · 1 line
Coverity-bot
#1 lore raw

Hi Benedikt,

Quoted 14 lines · 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:

Quoted 7 lines

*** 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.

Quoted 15 lines

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):

Quoted 6 lines

Addresses-Coverity-ID: 1527139 ("Control flow issues")

Fixes 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")

Thanks for your attention!

cheers,
-roger

#2 lore raw
Quoted 25 lines · Roger Quadros

Hi Benedikt,

Quoted 14 lines · 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:

Quoted 7 lines

*** 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)

Quoted 4 lines

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.

Quoted 16 lines
Quoted 16 lines

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):

Quoted 7 lines
Quoted 6 lines

Addresses-Coverity-ID: 1527139 ("Control flow issues")

Fixes 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")

Thanks for your attention!

cheers,
-roger

#3 lore raw
Quoted 27 lines · Roger Quadros
Quoted 25 lines · Roger Quadros

Hi Benedikt,

Quoted 21 lines · 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&amp;data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&amp;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)

Quoted 7 lines
Quoted 4 lines

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?

Quoted 20 lines
Quoted 20 lines
Quoted 20 lines

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):

Addresses-Coverity-ID: 1527139 ("Control flow issues")

Fixes 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

Thanks for your attention!

cheers,
-roger

cheers,
benedikt

#4 lore raw
Quoted 32 lines · Benedikt Niedermayr
Quoted 31 lines · Roger Quadros
Quoted 29 lines · Roger Quadros

Hi Benedikt,

Quoted 14 lines · 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&amp;data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&amp;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:

Quoted 7 lines

*** 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.

Quoted 6 lines
Quoted 2 lines

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?

Quoted 28 lines
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines

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):

Addresses-Coverity-ID: 1527139 ("Control flow issues")

Fixes 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

Thanks for your attention!

cheers,
-roger

cheers,
benedikt

#5 lore raw
Quoted 8 lines · BENEDIKT
Quoted 6 lines
Quoted 2 lines

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

#6 lore raw
In reply to Niedermayr, BENEDIKT
Quoted 33 lines · BENEDIKT
Quoted 32 lines · Roger Quadros
Quoted 29 lines · Roger Quadros

Hi Benedikt,

Quoted 14 lines · 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&amp;data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&amp;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:

Quoted 7 lines

*** 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.

Quoted 6 lines
Quoted 2 lines

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