Re: [PATCH v2 13/13] PCI/CXL: Refuse an SBR of a CXL DPort unless authorized

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl_sbr_unbind_regions() and cxl_sbr_enable_regions() ignore the return value of cxl_sbr_collect_regions().
--

--- Patch [13]: [PATCH v2 13/13] PCI/CXL: Refuse an SBR of a CXL DPort unless authorized ---
commit 8a167a59c2870bad236e079ff1641933532c8a30
Author: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

PCI/CXL: Refuse an SBR of a CXL DPort unless authorized

This commit prevents the Secondary Bus Reset (SBR) bit from being
silently unmasked if it is left at the firmware default. It adds
cxl_sbr_allowed() to verify whether a CXL Downstream Port has its
Unmask SBR bit set, requiring explicit authorization otherwise.

> diff --git a/drivers/cxl/core/dport_sbr.c b/drivers/cxl/core/dport_sbr.c
> new file mode 100644
> index 0000000000000..823b63012f45b
> --- /dev/null
> +++ b/drivers/cxl/core/dport_sbr.c

[ ... ]

> +static void cxl_sbr_unbind_regions(struct pci_dev *dport_pci)
> +{
> +	struct cxl_region *cxlr;
> +	struct xarray regions;
> +	unsigned long index;
> +
> +	if (cxl_sbr_save_hdm_state(dport_pci))
> +		pci_warn(dport_pci, "HDM state not saved, decode will not be restored\n");
> +
> +	xa_init(&regions);
> +
> +	cxl_sbr_collect_regions(dport_pci, &regions);

[Severity: High]
Does this code silently ignore memory allocation failures during region
collection?

If xa_insert() in cxl_sbr_collect_regions() returns -ENOMEM under memory
pressure, the error appears to be ignored, resulting in a partial unbind
before a bus reset.

Could this leave uncollected regions bound while the link is reset, leading
to Machine Check Exceptions if the memory is accessed while still online?

> +
> +	xa_for_each(&regions, index, cxlr)
> +		cxl_region_unbind(cxlr);
> +
> +	cxl_sbr_put_regions(&regions);
> +}

[ ... ]

> +static void cxl_sbr_enable_regions(struct pci_dev *dport_pci)
> +{
> +	struct xarray *hdm_state;
> +	struct cxl_region *cxlr;
> +	struct xarray regions;
> +	unsigned long index;
> +
> +	xa_init(&regions);
> +
> +	cxl_sbr_collect_regions(dport_pci, &regions);

[Severity: High]
Is the return value of cxl_sbr_collect_regions() intentionally ignored here
as well?

If region collection fails, it seems the uncollected regions would skip the
enable step and permanently leak their disabled state.

> +
> +	hdm_state = xa_load(&cxl_sbr_hdm_state, (unsigned long)dport_pci);
> +	if (hdm_state)
> +		cxl_sbr_recommit_decoders(dport_pci, hdm_state);
> +	else
> +		pci_warn(dport_pci, "no saved HDM state, decode not restored\n");
> +
> +	xa_for_each(&regions, index, cxlr)
> +		cxl_region_enable(cxlr);
> +
> +	cxl_sbr_put_regions(&regions);
> +	cxl_sbr_drop_hdm_state(dport_pci);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825022628.3651434-1-fabio.m.de.francesco@linux.intel.com?part=13