Re: [PATCH qemu] hw/cxl: Fix register block locator size

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Fri, 30 May 2025 02:59:40 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:

> On 29/05/2025 21:48, Jonathan Cameron via wrote:
> > This has been wrong from day 1.  For now we only have
> > two entries (component and device registers).  
> 
> Wow, I finally understood this.
> 
> 
> > 
> > The wrong size could lead to arbitrary data off the stack being presented
> > in PCIe config space.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >   include/hw/cxl/cxl_pci.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
> > index d0855ed78b..3bb882ce89 100644
> > --- a/include/hw/cxl/cxl_pci.h
> > +++ b/include/hw/cxl/cxl_pci.h
> > @@ -31,7 +31,7 @@
> >   #define PCIE_CXL3_FLEXBUS_PORT_DVSEC_LENGTH 0x20
> >   #define PCIE_CXL3_FLEXBUS_PORT_DVSEC_REVID  2
> >   
> > -#define REG_LOC_DVSEC_LENGTH 0x24
> > +#define REG_LOC_DVSEC_LENGTH 0x1C  
> 
> IMHO, REG_LOC_DVSEC_LENGTH is device specific, that mean we shouldn't put it in
> a general header with a general name
> 
> try:
> $ git grep REG_LOC_DVSEC_LENGTH
> 
> we got another REG_LOC_DVSEC_LENGTH, shouldn't its value (0x1C - 0x8)?
> 
> 
>   51     regloc_dvsec = &(CXLDVSECRegisterLocator) {
>   52         .rsvd         = 0,
>   53         .reg0_base_lo = RBI_CXL_DEVICE_REG | 0,
>   54         .reg0_base_hi = 0,
>   55     };
>   56     cxl_component_create_dvsec(cxl_cstate, CXL3_SWITCH_MAILBOX_CCI,
>   57                                REG_LOC_DVSEC_LENGTH, REG_LOC_DVSEC,
>   58                                REG_LOC_DVSEC_REVID, (uint8_t *)regloc_dvsec);
> 
Ah.  This isn't a bug at all.  I clearly needed more caffeine.

We are fine because at least in 3.2 the register block identifier of 0 is reserved and
I misread the code completely.  It is odd to have empty entries but not a bug.

Jonathan

> 
> Thanks
> Zhijian
> 
> 
> 
> >   #define REG_LOC_DVSEC_REVID  0
> >   
> >   enum