Coverity discovered a potential shift overflow in group size calculation
in the case of a guest error. Add checks and logs to ensure a issues are
caught.

Make the group and crowd error checking code more similar to one another
while here.

Resolves: Coverity CID 1593724
Fixes: 9cb7f6ebed60 ("ppc/xive2: Support group-matching when looking for target")
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/intc/xive.c  | 27 ++++++++++++++++++++++++---
 hw/intc/xive2.c | 19 ++++++++++++++-----
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index e86f2749328..3eb28c2265d 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1662,12 +1662,20 @@ uint32_t xive_get_vpgroup_size(uint32_t nvp_index)
      * (starting with the least significant bits) in the NVP index
      * gives the size of the group.
      */
-    return 1 << (ctz32(~nvp_index) + 1);
+    int first_zero = cto32(nvp_index);
+    if (first_zero >= 31) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
+                                       nvp_index);
+        return 0;
+    }
+
+    return 1U << (first_zero + 1);
 }
 
 static uint8_t xive_get_group_level(bool crowd, bool ignore,
                                     uint32_t nvp_blk, uint32_t nvp_index)
 {
+    int first_zero;
     uint8_t level;
 
     if (!ignore) {
@@ -1675,12 +1683,25 @@ static uint8_t xive_get_group_level(bool crowd, bool ignore,
         return 0;
     }
 
-    level = (ctz32(~nvp_index) + 1) & 0b1111;
+    first_zero = cto32(nvp_index);
+    if (first_zero >= 31) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
+                                       nvp_index);
+        return 0;
+    }
+
+    level = (first_zero + 1) & 0b1111;
     if (crowd) {
         uint32_t blk;
 
         /* crowd level is bit position of first 0 from the right in nvp_blk */
-        blk = ctz32(~nvp_blk) + 1;
+        first_zero = cto32(nvp_blk);
+        if (first_zero >= 31) {
+            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd block 0x%08x",
+                                           nvp_blk);
+            return 0;
+        }
+        blk = first_zero + 1;
 
         /*
          * Supported crowd sizes are 2^1, 2^2, and 2^4. 2^3 is not supported.
diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
index f8ef6154878..311b42e15d3 100644
--- a/hw/intc/xive2.c
+++ b/hw/intc/xive2.c
@@ -1153,13 +1153,15 @@ static bool xive2_vp_match_mask(uint32_t cam1, uint32_t cam2,
 
 static uint8_t xive2_get_vp_block_mask(uint32_t nvt_blk, bool crowd)
 {
-    uint8_t size, block_mask = 0b1111;
+    uint8_t block_mask = 0b1111;
 
     /* 3 supported crowd sizes: 2, 4, 16 */
     if (crowd) {
-        size = xive_get_vpgroup_size(nvt_blk);
-        if (size == 8) {
-            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of 8n");
+        uint32_t size = xive_get_vpgroup_size(nvt_blk);
+
+        if (size != 2 && size != 4 && size != 16) {
+            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of %d",
+                                           size);
             return block_mask;
         }
         block_mask &= ~(size - 1);
@@ -1172,7 +1174,14 @@ static uint32_t xive2_get_vp_index_mask(uint32_t nvt_index, bool cam_ignore)
     uint32_t index_mask = 0xFFFFFF; /* 24 bits */
 
     if (cam_ignore) {
-        index_mask &= ~(xive_get_vpgroup_size(nvt_index) - 1);
+        uint32_t size = xive_get_vpgroup_size(nvt_index);
+
+        if (size < 2) {
+            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group size of %d",
+                                           size);
+            return index_mask;
+        }
+        index_mask &= ~(size - 1);
     }
     return index_mask;
 }
-- 
2.47.1

On 3/17/25 06:23, Nicholas Piggin wrote:
> Coverity discovered a potential shift overflow in group size calculation
> in the case of a guest error. Add checks and logs to ensure a issues are
> caught.
> 
> Make the group and crowd error checking code more similar to one another
> while here.
> 
> Resolves: Coverity CID 1593724
> Fixes: 9cb7f6ebed60 ("ppc/xive2: Support group-matching when looking for target")
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/intc/xive.c  | 27 ++++++++++++++++++++++++---
>   hw/intc/xive2.c | 19 ++++++++++++++-----
>   2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index e86f2749328..3eb28c2265d 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1662,12 +1662,20 @@ uint32_t xive_get_vpgroup_size(uint32_t nvp_index)
>        * (starting with the least significant bits) in the NVP index
>        * gives the size of the group.
>        */
> -    return 1 << (ctz32(~nvp_index) + 1);
> +    int first_zero = cto32(nvp_index);
> +    if (first_zero >= 31) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
> +                                       nvp_index);
> +        return 0;
> +    }
> +
> +    return 1U << (first_zero + 1);
>   }
>   
>   static uint8_t xive_get_group_level(bool crowd, bool ignore,
>                                       uint32_t nvp_blk, uint32_t nvp_index)
>   {
> +    int first_zero;
>       uint8_t level;
>   
>       if (!ignore) {
> @@ -1675,12 +1683,25 @@ static uint8_t xive_get_group_level(bool crowd, bool ignore,
>           return 0;
>       }
>   
> -    level = (ctz32(~nvp_index) + 1) & 0b1111;
> +    first_zero = cto32(nvp_index);
> +    if (first_zero >= 31) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group index 0x%08x",
> +                                       nvp_index);
> +        return 0;
> +    }
> +
> +    level = (first_zero + 1) & 0b1111;
>       if (crowd) {
>           uint32_t blk;
>   
>           /* crowd level is bit position of first 0 from the right in nvp_blk */
> -        blk = ctz32(~nvp_blk) + 1;
> +        first_zero = cto32(nvp_blk);
> +        if (first_zero >= 31) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd block 0x%08x",
> +                                           nvp_blk);
> +            return 0;
> +        }
> +        blk = first_zero + 1;
>   
>           /*
>            * Supported crowd sizes are 2^1, 2^2, and 2^4. 2^3 is not supported.
> diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
> index f8ef6154878..311b42e15d3 100644
> --- a/hw/intc/xive2.c
> +++ b/hw/intc/xive2.c
> @@ -1153,13 +1153,15 @@ static bool xive2_vp_match_mask(uint32_t cam1, uint32_t cam2,
>   
>   static uint8_t xive2_get_vp_block_mask(uint32_t nvt_blk, bool crowd)
>   {
> -    uint8_t size, block_mask = 0b1111;
> +    uint8_t block_mask = 0b1111;
>   
>       /* 3 supported crowd sizes: 2, 4, 16 */
>       if (crowd) {
> -        size = xive_get_vpgroup_size(nvt_blk);
> -        if (size == 8) {
> -            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of 8n");
> +        uint32_t size = xive_get_vpgroup_size(nvt_blk);
> +
> +        if (size != 2 && size != 4 && size != 16) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid crowd size of %d",
> +                                           size);
>               return block_mask;
>           }
>           block_mask &= ~(size - 1);
> @@ -1172,7 +1174,14 @@ static uint32_t xive2_get_vp_index_mask(uint32_t nvt_index, bool cam_ignore)
>       uint32_t index_mask = 0xFFFFFF; /* 24 bits */
>   
>       if (cam_ignore) {
> -        index_mask &= ~(xive_get_vpgroup_size(nvt_index) - 1);
> +        uint32_t size = xive_get_vpgroup_size(nvt_index);
> +
> +        if (size < 2) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid group size of %d",
> +                                           size);
> +            return index_mask;
> +        }
> +        index_mask &= ~(size - 1);
>       }
>       return index_mask;
>   }