Re: [PATCH v29 2/5] cxl/sfc: Map cxl regs

["Dan Williams (nvidia)" <djbw@kernel.org>]

sashiko-bot@ wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] cxl_dport_map_rcd_linkcap() maps RCD link capability registers using devm on the endpoint's pci_dev, but stores the pointer in the longer-lived cxl_dport, creating a dangling pointer when the endpoint driver unbinds.
> --
> 
> commit 521897a4e4d35020c7cc57f7004dd3c659771f44
> Author: Alejandro Lucero <alucerop@amd.com>
> 
> cxl/sfc: Map cxl regs
> 
> This patch exports cxl core functions to allow a Type2 driver to discover
> and map device registers. It then utilizes these exported functions during
> the sfc driver's cxl initialization path.
> 
> > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> > index be252af972ab3..704b0ebae9374 100644
> > --- a/drivers/net/ethernet/sfc/efx_cxl.c
> > +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> [ ... ]
> > @@ -42,6 +45,29 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
> >  	if (!cxl)
> >  		return -ENOMEM;
> >  
> > +	rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
> > +				&cxl->cxlds.reg_map);
> 
> [Severity: High]
> This is a pre-existing issue, but does adding this call in a new endpoint
> driver expose us to a use-after-free when the endpoint driver unbinds?
> 
> Looking at the call chain, cxl_pci_setup_regs() can call
> cxl_dport_map_rcd_linkcap(). In that function, it allocates an I/O memory
> mapping bound to the endpoint device (pdev) via devres, but stores the
> pointer in the longer-lived dport structure:
> 
> drivers/cxl/core/regs.c:cxl_dport_map_rcd_linkcap() {
>     ...
>     dport_pcie_cap = devm_cxl_iomap_block(&pdev->dev, ...);
>     ...
>     dport->regs.rcd_pcie_cap = dport_pcie_cap;
> }
> 
> Since dport belongs to the upstream CXL port and outlives the pdev binding,
> if the sfc driver unbinds, the devm mapping is freed, leaving
> dport->regs.rcd_pcie_cap as a dangling pointer.
> 
> In a multi-function Restricted CXL Device (RCD), all functions share the
> same upstream host bridge and therefore the same cxl_dport. If multiple
> functions bind to drivers (e.g., cxl_pci and sfc) and sequentially
> overwrite the devres-managed mapping, could unbinding the most recently
> bound function cause a kernel oops later if a user reads the sysfs
> attribute via rcd_pcie_cap_emit()?

Yes, multi-function RCD needs explicit handling to either be marked
unsupported, or to move dport register mapping to a reference counted
arrangement. Likely the former as there are no known multi-function RCDs
and new hardware should be VH compatible. SFC is not an RCD.