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

[Miles Glenn]

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>