[PATCH v2] ppc/xive2: Fix integer overflow warning in xive2_redistribute()

Gautam Menghani posted 1 patch 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250811074912.162774-1-gautam@linux.ibm.com
Maintainers: Gautam Menghani <gautam@linux.ibm.com>
hw/intc/xive2.c | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)
[PATCH v2] ppc/xive2: Fix integer overflow warning in xive2_redistribute()
Posted by Gautam Menghani 3 months ago
Coverity reported an integer overflow warning in xive2_redistribute()
where the code does a left shift operation "0xffffffff << crowd". Fix the
warning by using a 64 byte integer type. Also refactor the calculation
into dedicated routines.

Resolves: Coverity CID 1612608
Fixes: 555e446019f5 ("ppc/xive2: Support redistribution of group interrupts")
Reviewed-by: Glenn Miles <milesg@linux.ibm.com>
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
v1 -> v2:
1. Remove inline keyword from the function definition (Glenn)

 hw/intc/xive2.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
index ee5fa26178..fbb3b7975e 100644
--- a/hw/intc/xive2.c
+++ b/hw/intc/xive2.c
@@ -95,6 +95,35 @@ static void xive2_nvgc_set_backlog(Xive2Nvgc *nvgc, uint8_t priority,
     }
 }
 
+static uint32_t xive2_nvgc_get_idx(uint32_t nvp_idx, uint8_t group)
+{
+    uint32_t nvgc_idx;
+
+    if (group > 0) {
+        nvgc_idx = (nvp_idx & (0xffffffffULL << group)) |
+                   ((1 << (group - 1)) - 1);
+    } else {
+        nvgc_idx = nvp_idx;
+    }
+
+    return nvgc_idx;
+}
+
+static uint8_t xive2_nvgc_get_blk(uint8_t nvp_blk, uint8_t crowd)
+{
+    uint8_t nvgc_blk;
+
+    if (crowd > 0) {
+        crowd = (crowd == 3) ? 4 : crowd;
+        nvgc_blk = (nvp_blk & (0xffffffffULL << crowd)) |
+                   ((1 << (crowd - 1)) - 1);
+    } else {
+        nvgc_blk = nvp_blk;
+    }
+
+    return nvgc_blk;
+}
+
 uint64_t xive2_presenter_nvgc_backlog_op(XivePresenter *xptr,
                                          bool crowd,
                                          uint8_t blk, uint32_t idx,
@@ -638,20 +667,8 @@ static void xive2_redistribute(Xive2Router *xrtr, XiveTCTX *tctx, uint8_t ring)
 
     trace_xive_redistribute(tctx->cs->cpu_index, ring, nvp_blk, nvp_idx);
     /* convert crowd/group to blk/idx */
-    if (group > 0) {
-        nvgc_idx = (nvp_idx & (0xffffffff << group)) |
-                   ((1 << (group - 1)) - 1);
-    } else {
-        nvgc_idx = nvp_idx;
-    }
-
-    if (crowd > 0) {
-        crowd = (crowd == 3) ? 4 : crowd;
-        nvgc_blk = (nvp_blk & (0xffffffff << crowd)) |
-                   ((1 << (crowd - 1)) - 1);
-    } else {
-        nvgc_blk = nvp_blk;
-    }
+    nvgc_idx = xive2_nvgc_get_idx(nvp_idx, group);
+    nvgc_blk = xive2_nvgc_get_blk(nvp_blk, crowd);
 
     /* Use blk/idx to retrieve the NVGC */
     if (xive2_router_get_nvgc(xrtr, crowd, nvgc_blk, nvgc_idx, &nvgc)) {
-- 
2.50.1
Re: [PATCH v2] ppc/xive2: Fix integer overflow warning in xive2_redistribute()
Posted by Amit Machhiwal 3 months ago
On 2025/08/11 01:19 PM, Gautam Menghani wrote:
> Coverity reported an integer overflow warning in xive2_redistribute()
> where the code does a left shift operation "0xffffffff << crowd". Fix the
> warning by using a 64 byte integer type. Also refactor the calculation
> into dedicated routines.
> 
> Resolves: Coverity CID 1612608
> Fixes: 555e446019f5 ("ppc/xive2: Support redistribution of group interrupts")
> Reviewed-by: Glenn Miles <milesg@linux.ibm.com>
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>

LGTM.

Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>

Thanks,
Amit

> ---
> v1 -> v2:
> 1. Remove inline keyword from the function definition (Glenn)
> 
>  hw/intc/xive2.c | 45 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/intc/xive2.c b/hw/intc/xive2.c
> index ee5fa26178..fbb3b7975e 100644
> --- a/hw/intc/xive2.c
> +++ b/hw/intc/xive2.c
> @@ -95,6 +95,35 @@ static void xive2_nvgc_set_backlog(Xive2Nvgc *nvgc, uint8_t priority,
>      }
>  }
>  
> +static uint32_t xive2_nvgc_get_idx(uint32_t nvp_idx, uint8_t group)
> +{
> +    uint32_t nvgc_idx;
> +
> +    if (group > 0) {
> +        nvgc_idx = (nvp_idx & (0xffffffffULL << group)) |
> +                   ((1 << (group - 1)) - 1);
> +    } else {
> +        nvgc_idx = nvp_idx;
> +    }
> +
> +    return nvgc_idx;
> +}
> +
> +static uint8_t xive2_nvgc_get_blk(uint8_t nvp_blk, uint8_t crowd)
> +{
> +    uint8_t nvgc_blk;
> +
> +    if (crowd > 0) {
> +        crowd = (crowd == 3) ? 4 : crowd;
> +        nvgc_blk = (nvp_blk & (0xffffffffULL << crowd)) |
> +                   ((1 << (crowd - 1)) - 1);
> +    } else {
> +        nvgc_blk = nvp_blk;
> +    }
> +
> +    return nvgc_blk;
> +}
> +
>  uint64_t xive2_presenter_nvgc_backlog_op(XivePresenter *xptr,
>                                           bool crowd,
>                                           uint8_t blk, uint32_t idx,
> @@ -638,20 +667,8 @@ static void xive2_redistribute(Xive2Router *xrtr, XiveTCTX *tctx, uint8_t ring)
>  
>      trace_xive_redistribute(tctx->cs->cpu_index, ring, nvp_blk, nvp_idx);
>      /* convert crowd/group to blk/idx */
> -    if (group > 0) {
> -        nvgc_idx = (nvp_idx & (0xffffffff << group)) |
> -                   ((1 << (group - 1)) - 1);
> -    } else {
> -        nvgc_idx = nvp_idx;
> -    }
> -
> -    if (crowd > 0) {
> -        crowd = (crowd == 3) ? 4 : crowd;
> -        nvgc_blk = (nvp_blk & (0xffffffff << crowd)) |
> -                   ((1 << (crowd - 1)) - 1);
> -    } else {
> -        nvgc_blk = nvp_blk;
> -    }
> +    nvgc_idx = xive2_nvgc_get_idx(nvp_idx, group);
> +    nvgc_blk = xive2_nvgc_get_blk(nvp_blk, crowd);
>  
>      /* Use blk/idx to retrieve the NVGC */
>      if (xive2_router_get_nvgc(xrtr, crowd, nvgc_blk, nvgc_idx, &nvgc)) {
> -- 
> 2.50.1
> 
>