Re: [RFC PATCH v2 1/2] PCI/CXL: Add RDPAS parsing support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The global `rdpas_parsed` flag is checked and updated without synchronization, leading to concurrent initialization races.
- [High] The per-RCEC container is published globally before its port list is populated, allowing lockless AER handlers to encounter corrupted or uninitialized list state.
--

commit 0749bd9917c3af37380ffaa0f749878e1c45c2ad
Author: Dave Jiang <dave.jiang@intel.com>
Subject: PCI/CXL: Add RDPAS parsing support

This commit introduces parsing for the RCEC Downstream Port Association
Structure (RDPAS) from the CEDT. The parsed entries are stored in a global
xarray that contains per-RCEC linked lists of downstream ports to help error
handlers locate them.

> diff --git a/drivers/pci/pcie/aer_cxl_rch.c b/drivers/pci/pcie/aer_cxl_rch.c
> index e471eefec9c40..5b0778fc08d7b 100644
> --- a/drivers/pci/pcie/aer_cxl_rch.c
> +++ b/drivers/pci/pcie/aer_cxl_rch.c

[ ... ]

> +static int __cxl_parse_rdpas(struct acpi_cedt_rdpas *rdpas, struct device *dev)
> +{
> +	struct cxl_rdpas_rcec *rdpas_rcec;
> +	struct cxl_rdpas_entry *entry;
> +	unsigned long index;
> +	int rc;

[ ... ]

> +	index = __rdpas_index(rdpas->segment, rdpas->bdf);
> +
> +	rdpas_rcec = xa_load(&cxl_rdpas, index);
> +	if (!rdpas_rcec) {
> +		rdpas_rcec = kzalloc(sizeof(*rdpas_rcec), GFP_KERNEL);
> +		if (!rdpas_rcec)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&rdpas_rcec->ports);
> +		rc = xa_insert(&cxl_rdpas, index, rdpas_rcec, GFP_KERNEL);
> +		if (rc) {
> +			kfree(rdpas_rcec);
> +			return rc;
> +		}
> +	}
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->address = rdpas->address;
> +	entry->protocol = rdpas->protocol;
> +	list_add_tail(&entry->list, &rdpas_rcec->ports);

[Severity: High]
Is it safe to publish the rdpas_rcec container to the global cxl_rdpas
xarray via xa_insert() before its port list is populated?

If a concurrent AER error handler retrieves this container locklessly using
cxl_get_rdpas_by_rcec(), it might iterate the list using
list_for_each_entry(). Because list_add_tail() is used without RCU memory
barriers, a concurrent reader could observe an uninitialized or partially
initialized list state, which could result in a NULL pointer dereference
or crash.

[ ... ]

> +int cxl_rdpas_init(struct device *host)
> +{
> +	if (rdpas_parsed)
> +		return 0;
> +
> +	rdpas_parsed = true;

[Severity: High]
Could there be a race condition when checking and setting rdpas_parsed
without synchronization?

If multiple threads probe cxl_acpi concurrently (for example, via sysfs bind
or if multiple ACPI0017 devices exist), they could both observe
rdpas_parsed == false. This would cause them to concurrently execute
acpi_table_parse_cedt() and perform lockless list_add_tail() operations
for the same RCEC container, potentially corrupting the list pointers.

> +
> +	return acpi_table_parse_cedt(ACPI_CEDT_TYPE_RDPAS, cxl_parse_rdpas, host);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618170723.2010490-1-dave.jiang@intel.com?part=1