[v1] CXL emulation fixes and minor cleanup.

[PATCH 09/12] hw/mem/cxl_type3: Fix potential divide by zero reported by coverity

Posted by Jonathan Cameron via 10 months, 1 week ago

Fixes Coverity ID 1522368.

Currently error_fatal is set if interleave_ways_dec() is going to return 0
but we should handle that zero return explicitly.

Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
Note this is a stop gap until a more complex HDM decoder verification series.
---
 hw/mem/cxl_type3.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 1b92a065a3..24211703c6 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -794,8 +794,12 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr host_addr, uint64_t *dpa)
         }
         if (((uint64_t)host_addr < decoder_base) ||
             (hpa_offset >= decoder_size)) {
-            dpa_base += decoder_size /
-                cxl_interleave_ways_dec(iw, &error_fatal);
+            int decoded_iw = cxl_interleave_ways_dec(iw, &error_fatal);
+
+            if (decoded_iw == 0)
+                return false;
+
+            dpa_base += decoder_size / decoded_iw;
             continue;
         }
 
-- 
2.39.2

Re: [PATCH 09/12] hw/mem/cxl_type3: Fix potential divide by zero reported by coverity

Posted by fan 10 months ago

On Wed, Jan 24, 2024 at 12:40:57PM +0000, Jonathan Cameron wrote:
> Fixes Coverity ID 1522368.
> 
> Currently error_fatal is set if interleave_ways_dec() is going to return 0
> but we should handle that zero return explicitly.
> 
> Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 

Reviewed-by: Fan Ni <fan.ni@samsung.com>

> ---
> Note this is a stop gap until a more complex HDM decoder verification series.
> ---
>  hw/mem/cxl_type3.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 1b92a065a3..24211703c6 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -794,8 +794,12 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr host_addr, uint64_t *dpa)
>          }
>          if (((uint64_t)host_addr < decoder_base) ||
>              (hpa_offset >= decoder_size)) {
> -            dpa_base += decoder_size /
> -                cxl_interleave_ways_dec(iw, &error_fatal);
> +            int decoded_iw = cxl_interleave_ways_dec(iw, &error_fatal);
> +
> +            if (decoded_iw == 0)
> +                return false;
> +
> +            dpa_base += decoder_size / decoded_iw;
>              continue;
>          }
>  
> -- 
> 2.39.2
>

Re: [PATCH 09/12] hw/mem/cxl_type3: Fix potential divide by zero reported by coverity

Posted by Jonathan Cameron via 10 months ago

On Wed, 24 Jan 2024 15:53:58 -0800
fan <nifan.cxl@gmail.com> wrote:

> On Wed, Jan 24, 2024 at 12:40:57PM +0000, Jonathan Cameron wrote:
> > Fixes Coverity ID 1522368.
> > 
> > Currently error_fatal is set if interleave_ways_dec() is going to return 0
> > but we should handle that zero return explicitly.
> > 
> > Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >   
> 
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> 
> > ---
> > Note this is a stop gap until a more complex HDM decoder verification series.
> > ---
> >  hw/mem/cxl_type3.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index 1b92a065a3..24211703c6 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -794,8 +794,12 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr host_addr, uint64_t *dpa)
> >          }
> >          if (((uint64_t)host_addr < decoder_base) ||
> >              (hpa_offset >= decoder_size)) {
> > -            dpa_base += decoder_size /
> > -                cxl_interleave_ways_dec(iw, &error_fatal);
> > +            int decoded_iw = cxl_interleave_ways_dec(iw, &error_fatal);
> > +
> > +            if (decoded_iw == 0)
> > +                return false;
I've obviously been lax in running checkpatch.
Brackets needed for qemu style here. I'll add them for v2

Jonathan

> > +
> > +            dpa_base += decoder_size / decoded_iw;
> >              continue;
> >          }
> >  
> > -- 
> > 2.39.2
> >