1. Patchew
  2. QEMU
  3. View series

[PATCH] misc/pca9554: Fix check of pin range value in property accessors

Cédric Le Goater posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240321160154.901829-1-clg@redhat.com
Maintainers: Glenn Miles <milesg@linux.vnet.ibm.com>
hw/misc/pca9554.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] misc/pca9554: Fix check of pin range value in property accessors
Posted by Cédric Le Goater 1 month, 1 week ago 
Coverity detected an "Integer handling" issue with the pin value :

  In expression "state >> pin", right shifting "state" by more than 7
  bits always yields zero.  The shift amount, "pin", is as much as 8.

In practice, this should not happen because the properties "pin8" and
above are not created. Nevertheless, fix the range to avoid this warning.

Fixes: CID 1534917
Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
Cc: Glenn Miles <milesg@linux.vnet.ibm.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/misc/pca9554.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
index 778b32e4430a8b618322c26b1b185ed3ead97cc4..5e31696797d9564666ece6fe17737ee2a9733e96 100644
--- a/hw/misc/pca9554.c
+++ b/hw/misc/pca9554.c
@@ -160,7 +160,7 @@ static void pca9554_get_pin(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
+    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
         error_setg(errp, "%s invalid pin %s", __func__, name);
         return;
     }
@@ -187,7 +187,7 @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
+    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
         error_setg(errp, "%s invalid pin %s", __func__, name);
         return;
     }
-- 
2.44.0
Re: [PATCH] misc/pca9554: Fix check of pin range value in property accessors
Posted by Philippe Mathieu-Daudé 1 month, 1 week ago 
On 21/3/24 17:01, Cédric Le Goater wrote:
> Coverity detected an "Integer handling" issue with the pin value :
> 
>    In expression "state >> pin", right shifting "state" by more than 7
>    bits always yields zero.  The shift amount, "pin", is as much as 8.
> 
> In practice, this should not happen because the properties "pin8" and
> above are not created. Nevertheless, fix the range to avoid this warning.
> 
> Fixes: CID 1534917
> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
> Cc: Glenn Miles <milesg@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/misc/pca9554.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Isn't it the one Peter fixed in
https://lore.kernel.org/qemu-devel/20240312183810.557768-5-peter.maydell@linaro.org/?
Re: [PATCH] misc/pca9554: Fix check of pin range value in property accessors
Posted by Cédric Le Goater 1 month ago 
On 3/21/24 18:15, Philippe Mathieu-Daudé wrote:
> On 21/3/24 17:01, Cédric Le Goater wrote:
>> Coverity detected an "Integer handling" issue with the pin value :
>>
>>    In expression "state >> pin", right shifting "state" by more than 7
>>    bits always yields zero.  The shift amount, "pin", is as much as 8.
>>
>> In practice, this should not happen because the properties "pin8" and
>> above are not created. Nevertheless, fix the range to avoid this warning.
>>
>> Fixes: CID 1534917
>> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
>> Cc: Glenn Miles <milesg@linux.vnet.ibm.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/misc/pca9554.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Isn't it the one Peter fixed in
> https://lore.kernel.org/qemu-devel/20240312183810.557768-5-peter.maydell@linaro.org/?

Oh yes. I missed it. Hopefully, they are similar. Let's keep Peter's.

However, what I would like to do as a follow-up is to move the
hw/misc/pca955* models under hw/gpio/. Is it something we can do
for 9.0 ?

Thanks,

C.
Re: [PATCH] misc/pca9554: Fix check of pin range value in property accessors
Posted by Miles Glenn 1 month, 1 week ago 
On Thu, 2024-03-21 at 17:01 +0100, Cédric Le Goater wrote:
> Coverity detected an "Integer handling" issue with the pin value :
> 
>   In expression "state >> pin", right shifting "state" by more than 7
>   bits always yields zero.  The shift amount, "pin", is as much as 8.
> 
> In practice, this should not happen because the properties "pin8" and
> above are not created. Nevertheless, fix the range to avoid this
> warning.
> 
> Fixes: CID 1534917
> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
> Cc: Glenn Miles <milesg@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/misc/pca9554.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
> index
> 778b32e4430a8b618322c26b1b185ed3ead97cc4..5e31696797d9564666ece6fe177
> 37ee2a9733e96 100644
> --- a/hw/misc/pca9554.c
> +++ b/hw/misc/pca9554.c
> @@ -160,7 +160,7 @@ static void pca9554_get_pin(Object *obj, Visitor
> *v, const char *name,
>          error_setg(errp, "%s: error reading %s", __func__, name);
>          return;
>      }
> -    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
> +    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
>          error_setg(errp, "%s invalid pin %s", __func__, name);
>          return;
>      }
> @@ -187,7 +187,7 @@ static void pca9554_set_pin(Object *obj, Visitor
> *v, const char *name,
>          error_setg(errp, "%s: error reading %s", __func__, name);
>          return;
>      }
> -    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
> +    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
>          error_setg(errp, "%s invalid pin %s", __func__, name);
>          return;
>      }

Thanks, Cédric!  I guess I should be running coverity myself.

-Glenn

Reviewed-by: Glenn Miles <milesg@linux.vnet.ibm.com>
Re: [PATCH] misc/pca9554: Fix check of pin range value in property accessors
Posted by Cédric Le Goater 1 month, 1 week ago 
On 3/21/24 17:08, Miles Glenn wrote:
> On Thu, 2024-03-21 at 17:01 +0100, Cédric Le Goater wrote:
>> Coverity detected an "Integer handling" issue with the pin value :
>>
>>    In expression "state >> pin", right shifting "state" by more than 7
>>    bits always yields zero.  The shift amount, "pin", is as much as 8.
>>
>> In practice, this should not happen because the properties "pin8" and
>> above are not created. Nevertheless, fix the range to avoid this
>> warning.
>>
>> Fixes: CID 1534917
>> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
>> Cc: Glenn Miles <milesg@linux.vnet.ibm.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/misc/pca9554.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
>> index
>> 778b32e4430a8b618322c26b1b185ed3ead97cc4..5e31696797d9564666ece6fe177
>> 37ee2a9733e96 100644
>> --- a/hw/misc/pca9554.c
>> +++ b/hw/misc/pca9554.c
>> @@ -160,7 +160,7 @@ static void pca9554_get_pin(Object *obj, Visitor
>> *v, const char *name,
>>           error_setg(errp, "%s: error reading %s", __func__, name);
>>           return;
>>       }
>> -    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
>> +    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
>>           error_setg(errp, "%s invalid pin %s", __func__, name);
>>           return;
>>       }
>> @@ -187,7 +187,7 @@ static void pca9554_set_pin(Object *obj, Visitor
>> *v, const char *name,
>>           error_setg(errp, "%s: error reading %s", __func__, name);
>>           return;
>>       }
>> -    if (pin < 0 || pin > PCA9554_PIN_COUNT) {
>> +    if (pin < 0 || pin >= PCA9554_PIN_COUNT) {
>>           error_setg(errp, "%s invalid pin %s", __func__, name);
>>           return;
>>       }
> 
> Thanks, Cédric!  I guess I should be running coverity myself.

I don't myself. I get reports from :

   https://scan.coverity.com/projects/qemu

Thanks,

C.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.