hw/gpio/pca9552: fix off-by-one in QOM led index validation

[PATCH] hw/gpio/pca9552: fix off-by-one in QOM led index validation

Posted by yujun 3 weeks, 6 days ago

pca955x_get_led() and pca955x_set_led() accept led indices equal to
pin_count, but valid indices are 0..pin_count-1.  For a 16-pin device,
led16 passes the current check and then accesses an LS register past
max_reg.

Use the same >= pin_count bounds check as pca9554_set_pin() and the
gpio input handler assert in this file.

Fixes: a90d8f84674 ("misc/pca9552: Add qom set and get")
Signed-off-by: yujun <yujun@kylinos.cn>
---
 hw/gpio/pca9552.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/gpio/pca9552.c b/hw/gpio/pca9552.c
index 472d8ad957..b13ac9fd9c 100644
--- a/hw/gpio/pca9552.c
+++ b/hw/gpio/pca9552.c
@@ -311,8 +311,8 @@ static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (led < 0 || led > k->pin_count) {
-        error_setg(errp, "%s invalid led %s", __func__, name);
+    if (led < 0 || led >= k->pin_count) {
+        error_setg(errp, "%s: invalid led %s", __func__, name);
         return;
     }
     /*
@@ -352,8 +352,8 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
         error_setg(errp, "%s: error reading %s", __func__, name);
         return;
     }
-    if (led < 0 || led > k->pin_count) {
-        error_setg(errp, "%s invalid led %s", __func__, name);
+    if (led < 0 || led >= k->pin_count) {
+        error_setg(errp, "%s: invalid led %s", __func__, name);
         return;
     }
 
-- 
2.25.1

Re: [PATCH] hw/gpio/pca9552: fix off-by-one in QOM led index validation

Posted by Philippe Mathieu-Daudé 3 weeks, 5 days ago

On 29/6/26 09:41, yujun wrote:
> pca955x_get_led() and pca955x_set_led() accept led indices equal to
> pin_count, but valid indices are 0..pin_count-1.  For a 16-pin device,
> led16 passes the current check and then accesses an LS register past
> max_reg.
> 
> Use the same >= pin_count bounds check as pca9554_set_pin() and the
> gpio input handler assert in this file.
> 
> Fixes: a90d8f84674 ("misc/pca9552: Add qom set and get")
> Signed-off-by: yujun <yujun@kylinos.cn>
> ---
>   hw/gpio/pca9552.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)


Queued via hw-misc, thanks!

Phil.

Re: [PATCH] hw/gpio/pca9552: fix off-by-one in QOM led index validation

Posted by Miles Glenn 3 weeks, 5 days ago

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Mon, 2026-06-29 at 15:41 +0800, yujun wrote:
> pca955x_get_led() and pca955x_set_led() accept led indices equal to
> pin_count, but valid indices are 0..pin_count-1.  For a 16-pin device,
> led16 passes the current check and then accesses an LS register past
> max_reg.
> 
> Use the same >= pin_count bounds check as pca9554_set_pin() and the
> gpio input handler assert in this file.
> 
> Fixes: a90d8f84674 ("misc/pca9552: Add qom set and get")
> Signed-off-by: yujun <yujun@kylinos.cn>
> ---
>  hw/gpio/pca9552.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/gpio/pca9552.c b/hw/gpio/pca9552.c
> index 472d8ad957..b13ac9fd9c 100644
> --- a/hw/gpio/pca9552.c
> +++ b/hw/gpio/pca9552.c
> @@ -311,8 +311,8 @@ static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
>          error_setg(errp, "%s: error reading %s", __func__, name);
>          return;
>      }
> -    if (led < 0 || led > k->pin_count) {
> -        error_setg(errp, "%s invalid led %s", __func__, name);
> +    if (led < 0 || led >= k->pin_count) {
> +        error_setg(errp, "%s: invalid led %s", __func__, name);
>          return;
>      }
>      /*
> @@ -352,8 +352,8 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
>          error_setg(errp, "%s: error reading %s", __func__, name);
>          return;
>      }
> -    if (led < 0 || led > k->pin_count) {
> -        error_setg(errp, "%s invalid led %s", __func__, name);
> +    if (led < 0 || led >= k->pin_count) {
> +        error_setg(errp, "%s: invalid led %s", __func__, name);
>          return;
>      }
>

Re: [PATCH] hw/gpio/pca9552: fix off-by-one in QOM led index validation

Posted by Peter Maydell 3 weeks, 6 days ago

On Mon, 29 Jun 2026 at 08:42, yujun <yujun@kylinos.cn> wrote:
>
> pca955x_get_led() and pca955x_set_led() accept led indices equal to
> pin_count, but valid indices are 0..pin_count-1.  For a 16-pin device,
> led16 passes the current check and then accesses an LS register past
> max_reg.
>
> Use the same >= pin_count bounds check as pca9554_set_pin() and the
> gpio input handler assert in this file.
>
> Fixes: a90d8f84674 ("misc/pca9552: Add qom set and get")
> Signed-off-by: yujun <yujun@kylinos.cn>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH] hw/gpio/pca9552: fix off-by-one in QOM led index validation

Posted by Cédric Le Goater 3 weeks, 6 days ago

Glenn,

Cc: me if you ack this change. I can take it through the aspeed queue.

Thanks,

C.


On 6/29/26 09:41, yujun wrote:
> pca955x_get_led() and pca955x_set_led() accept led indices equal to
> pin_count, but valid indices are 0..pin_count-1.  For a 16-pin device,
> led16 passes the current check and then accesses an LS register past
> max_reg.
> 
> Use the same >= pin_count bounds check as pca9554_set_pin() and the
> gpio input handler assert in this file.
> 
> Fixes: a90d8f84674 ("misc/pca9552: Add qom set and get")
> Signed-off-by: yujun <yujun@kylinos.cn>
> ---
>   hw/gpio/pca9552.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/gpio/pca9552.c b/hw/gpio/pca9552.c
> index 472d8ad957..b13ac9fd9c 100644
> --- a/hw/gpio/pca9552.c
> +++ b/hw/gpio/pca9552.c
> @@ -311,8 +311,8 @@ static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
>           error_setg(errp, "%s: error reading %s", __func__, name);
>           return;
>       }
> -    if (led < 0 || led > k->pin_count) {
> -        error_setg(errp, "%s invalid led %s", __func__, name);
> +    if (led < 0 || led >= k->pin_count) {
> +        error_setg(errp, "%s: invalid led %s", __func__, name);
>           return;
>       }
>       /*
> @@ -352,8 +352,8 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
>           error_setg(errp, "%s: error reading %s", __func__, name);
>           return;
>       }
> -    if (led < 0 || led > k->pin_count) {
> -        error_setg(errp, "%s invalid led %s", __func__, name);
> +    if (led < 0 || led >= k->pin_count) {
> +        error_setg(errp, "%s: invalid led %s", __func__, name);
>           return;
>       }
>