Re: [PATCH] hw/cxl: Fix out of bound array access

[Jonathan Cameron]

On Wed, 13 Sep 2023 13:10:56 +0300
Dmitry Frolov <frolov@swemel.ru> wrote:

> According to cxl_interleave_ways_enc(),
> fw->num_targets is allowed to be up to 16.
> This also corresponds to CXL specs.
> So, the fw->target_hbs[] array is iterated from 0 to 15.
> But it is staticaly declared of length 8.
> Thus, out of bound array access may occur.
> 
> Fixes: c28db9e000 ("hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from 
> TYPE_PXB_DEV")
> 
> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>

Hi Dmitry,

Good spot - though I'm curious on whether you hit this in a 16 way interleave 
test and
hence care about this case?  My tests tend to burn the available ways in the 
topology
rather than doing a flat 16 way host interleave (which would be a crazy 
physical system
- I want one of those :)

This looks to be a missed update when we expanded the decoded number of 
interleave ways.
I think (looking at published ECNs) that occurred in a CXL r2.0 ECN dated Oct 
2021.
The CFWMS table was introduced as an ECN published in May 2021.  I'll note the 
r3.0
spec is confusing because CFMWS refers to the HDM decoder spec that says the 
values
beyond 1,2,4,8 are for endpoints only and this isn't one.  Examples make it 
clear
that rule doesn't apply though.

I suspect this bug was introduced whilst the code was still out of tree so hard 
to point at
when.

Anyhow, I'll queue this one or Michael can pick it up directly if he'd prefer.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---
>  include/hw/cxl/cxl.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index 56c9e7676e..4944725849 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -29,7 +29,7 @@ typedef struct PXBCXLDev PXBCXLDev;
>  typedef struct CXLFixedWindow {
>      uint64_t size;
>      char **targets;
> -    PXBCXLDev *target_hbs[8];
> +    PXBCXLDev *target_hbs[16];
>      uint8_t num_targets;
>      uint8_t enc_int_ways;
>      uint8_t enc_int_gran;