Coverity reports a possible memory overflow in spapr_dt_pa_features().
This should not be a true bug since DAWR1 cap is only be true for
CPU_POWERPC_LOGICAL_3_10. Add an assertion to ensure any bug there is
caught.

Resolves: Coverity CID 1593722
Fixes: 5f361ea187ba ("ppc: spapr: Enable 2nd DAWR on Power10 pSeries machine")
Cc: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a415e51d077..9865d7147ff 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -296,6 +296,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         pa_features[40 + 2] &= ~0x80; /* Radix MMU */
     }
     if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
+        g_assert(pa_size > 66);
         pa_features[66] |= 0x80;
     }
 
-- 
2.47.1

Hi Nick,


Thnks for taking care of this, I was about to post a fix.


On 3/17/25 10:53 AM, Nicholas Piggin wrote:
> Coverity reports a possible memory overflow in spapr_dt_pa_features().
> This should not be a true bug since DAWR1 cap is only be true for
> CPU_POWERPC_LOGICAL_3_10. Add an assertion to ensure any bug there is
> caught.
>
> Resolves: Coverity CID 1593722
> Fixes: 5f361ea187ba ("ppc: spapr: Enable 2nd DAWR on Power10 pSeries machine")
> Cc: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a415e51d077..9865d7147ff 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -296,6 +296,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           pa_features[40 + 2] &= ~0x80; /* Radix MMU */
>       }
>       if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
> +        g_assert(pa_size > 66);

SPAPR_CAP_DAWR1 is set to off in default_caps_with_cpu() for below P10 
(compat mode

cases).


So, this works.


I was planning to fix this instead with

+ if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1) && pa_size > 66) { 
pa_features[66] |= 0x80; }

just being in-line with the other similar checks in the same function.


Reviewed-By: Shivaprasad G Bhat <sbhat@linux.ibm.com>


Regards,

Shivaprasad

On 3/17/25 06:23, Nicholas Piggin wrote:
> Coverity reports a possible memory overflow in spapr_dt_pa_features().
> This should not be a true bug since DAWR1 cap is only be true for
> CPU_POWERPC_LOGICAL_3_10. Add an assertion to ensure any bug there is
> caught.
> 
> Resolves: Coverity CID 1593722
> Fixes: 5f361ea187ba ("ppc: spapr: Enable 2nd DAWR on Power10 pSeries machine")
> Cc: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a415e51d077..9865d7147ff 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -296,6 +296,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           pa_features[40 + 2] &= ~0x80; /* Radix MMU */
>       }
>       if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
> +        g_assert(pa_size > 66);
>           pa_features[66] |= 0x80;
>       }
>   

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

Thanks,

C.