[PATCH v4 0/2] cxl: Fix uninitialized access coordinates

[PATCH v4 0/2] cxl: Fix uninitialized access coordinates

[Guixin Liu]

Two fixes for uninitialized access_coordinate reads found by inspecting
the CXL bandwidth calculation paths.

Patch 1 zeroes the coordinate arrays that cxl_endpoint_gather_bandwidth()
and cxl_switch_gather_bandwidth() declare on the stack. Patch 2 zeroes
the output array of cxl_endpoint_get_perf_coordinates() when it returns
early for a Restricted CXL Device.

Testing:

Patch 1 was tested on a QEMU CXL topology with a switch and two volatile
endpoints sharing the switch upstream link. The kernel needs HMAT
generic-port coordinates for the host bridge to run the calculation at
all: without them the host bridge dport coordinates stay empty,
cxl_endpoint_get_perf_coordinates() returns -EINVAL, the endpoint DPA
perf is never populated, and cxled_get_dpa_perf() fails before either
gather function touches its arrays.

  -machine q35,accel=kvm,cxl=on,hmat=on
  -object acpi-generic-port,id=gp0,pci-bus=cxl.0,node=1
  -numa hmat-lb,initiator=0,target=1,hierarchy=memory,\
data-type=access-latency,latency=100
  -numa hmat-lb,initiator=0,target=1,hierarchy=memory,\
data-type=access-bandwidth,bandwidth=1G

  cxl create-region -d decoder0.1 -m mem0 mem1 -t ram -s 2G -w 2 -g 256

With a temporary printk added after cxl_pci_get_bandwidth() and after
cxl_coordinates_combine(), an unpatched kernel
(CONFIG_INIT_STACK_ALL_PATTERN) printed:

  REPRO ep_gather 0000:35:00.0: after cxl_pci_get_bandwidth
      pci_coord[LOCAL] rd_lat=0xfefefefe wr_lat=0xfefefefe
  REPRO ep_gather 0000:35:00.0: after combine
      ep_coord[LOCAL] rd_lat=0xfefeff94 wr_lat=0xfefefff8
  REPRO sw_gather 0000:36:00.0: after cxl_pci_get_bandwidth
      coords[LOCAL] rd_lat=0xfefefefe

The latency members read back as the pattern-init stack filler, and the
combine step sums that residue into ep_coord. With the patch the same
probes read 0x00000000 before the combine and the CDAT latency values
(0x96 / 0xfa, the 150/250 ns QEMU puts in DSLBIS) after it.

Not covered: the bandwidth members keeping residue into the region sysfs
attributes requires an endpoint whose CDAT reports zero bandwidth for an
access class. QEMU synthesizes non-zero DSLBIS values by default, and
supplying a custom CDAT with a zero entry was not done.

Patch 2 is not tested: reaching the path requires an RCD, which needs a
CEDT CHBS of the CXL 1.1 version, and QEMU only emits CXL 2.0 CHBS
entries and has no RCD device model.

v3 -> v4:
- rework the changelogs to state the failing condition, the consequence
  and the fix rather than narrate the walk through CDAT parsing,
  __cxl_coordinates_combine(), QoS class selection, cxl_dpa_perf and
  sysfs (Alison Schofield)
- say how each issue was found and how each patch was tested
  (Alison Schofield)
- drop the -EEXIST claim and the rest of the v2 framing that assumed
  cxl_pmem could be unloaded or unbound

v3:
https://lore.kernel.org/linux-cxl/\
20260812083035.372308-1-kanie@linux.alibaba.com/

Guixin Liu (2):
  cxl/cdat: Fix uninitialized stack use in bandwidth gathering
  cxl/port: Fix uninitialized coordinates reported for RCDs

 drivers/cxl/core/cdat.c | 8 ++++----
 drivers/cxl/core/port.c | 4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)


base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7

[PATCH v4 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[Guixin Liu]

cxl_endpoint_gather_bandwidth() and cxl_switch_gather_bandwidth() declare
access_coordinate arrays on the stack and rely on their helpers to fill
them. The helpers assign only the bandwidth members, and the combine step
assigns even those only when both inputs are non-zero. The latency members
are summed from stack residue on every call, and the bandwidth members
keep that residue into the region access coordinate sysfs attributes when
the endpoint CDAT reports no bandwidth.

Zero initialize the arrays. Zero is the value this code already uses for
"not reported", so an unset member now reads back as unknown rather than as
a plausible number.

Found by code inspection. Tested on a QEMU CXL topology with two endpoints
sharing a switch upstream link and HMAT generic-port coordinates for the
host bridge, which the calculation requires to run at all: with temporary
printk at the combine sites, the unpatched kernel summed 0xfefefefe, the
CONFIG_INIT_STACK_ALL_PATTERN stack filler, into the latency members, while
the patched kernel reports the CDAT latency values there.

Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/cdat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 5c9f07262513..df9261ba84e5 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -633,9 +633,9 @@ static int cxl_endpoint_gather_bandwidth(struct cxl_region *cxlr,
 	struct cxl_port *endpoint = to_cxl_port(cxled->cxld.dev.parent);
 	struct cxl_port *parent_port = to_cxl_port(endpoint->dev.parent);
 	struct cxl_port *gp_port = to_cxl_port(parent_port->dev.parent);
-	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX];
-	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX];
-	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX];
+	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX] = { };
+	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX] = { };
+	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX] = { };
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
@@ -757,7 +757,7 @@ static struct xarray *cxl_switch_gather_bandwidth(struct cxl_region *cxlr,
 {
 	struct xarray *res_xa __free(free_perf_xa) =
 		kzalloc_obj(*res_xa);
-	struct access_coordinate coords[ACCESS_COORDINATE_MAX];
+	struct access_coordinate coords[ACCESS_COORDINATE_MAX] = { };
 	struct cxl_perf_ctx *ctx, *us_ctx;
 	unsigned long index, us_index;
 	int dev_count = 0;
-- 
2.43.7

Re: [PATCH v4 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Inconsistent handling of 0 (unreported) bandwidth in __cxl_coordinates_combine due to pointer aliasing causes mathematically incorrect bandwidth calculations.
--

commit 4a06d73ff1f865478d3669ce819144aacd32799a
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/cdat: Fix uninitialized stack use in bandwidth gathering
    
The commit zeroes out access_coordinate arrays on the stack in
cxl_endpoint_gather_bandwidth() and cxl_switch_gather_bandwidth(). This
prevents uninitialized stack residues from being summed into the latency
members and stored in sysfs when the endpoint CDAT reports no bandwidth.

> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 5c9f072625135..df9261ba84e5a 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
[ ... ]
> @@ -757,7 +757,7 @@ static struct xarray *cxl_switch_gather_bandwidth(struct cxl_region *cxlr,
>  {
>  	struct xarray *res_xa __free(free_perf_xa) =
>  		kzalloc_obj(*res_xa);
> -	struct access_coordinate coords[ACCESS_COORDINATE_MAX];
> +	struct access_coordinate coords[ACCESS_COORDINATE_MAX] = { };
>  	struct cxl_perf_ctx *ctx, *us_ctx;
>  	unsigned long index, us_index;
>  	int dev_count = 0;

[Severity: High]
This is a pre-existing issue, but does pointer aliasing in
cxl_coordinates_combine() cause incorrect bandwidth calculations when an
endpoint reports 0 (unknown) bandwidth?

Looking at cxl_switch_gather_bandwidth(), if ctx->coord has 0 bandwidth,
does coords incorrectly retain the PCI link bandwidth instead of 0?

drivers/cxl/core/cdat.c:__cxl_coordinates_combine() {
    ...
		if (c1->write_bandwidth && c2->write_bandwidth)
			out->write_bandwidth = min(c1->write_bandwidth,
						   c2->write_bandwidth);
    ...
}

When called later in cxl_switch_gather_bandwidth(), coords aliases both
out and c1:

    /* Retrieve the upstream link bandwidth */
    rc = cxl_pci_get_bandwidth(to_pci_dev(dev), coords);
    ...
    cxl_coordinates_combine(coords, coords, ctx->coord);

Because coords is populated by cxl_pci_get_bandwidth(), it is non-zero.
If ctx->coord (c2) has 0 bandwidth, the condition fails and coords (out)
is left unmodified.

Will this cause the endpoint's bandwidth to be incorrectly aggregated up
the tree as the full PCI link bandwidth instead of 0 when CDAT data is
missing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831092216.540644-1-kanie@linux.alibaba.com?part=1

Re: [PATCH v4 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[Dave Jiang]

On 8/31/26 2:22 AM, Guixin Liu wrote:
> cxl_endpoint_gather_bandwidth() and cxl_switch_gather_bandwidth() declare
> access_coordinate arrays on the stack and rely on their helpers to fill
> them. The helpers assign only the bandwidth members, and the combine step
> assigns even those only when both inputs are non-zero. The latency members
> are summed from stack residue on every call, and the bandwidth members
> keep that residue into the region access coordinate sysfs attributes when
> the endpoint CDAT reports no bandwidth.
> 
> Zero initialize the arrays. Zero is the value this code already uses for
> "not reported", so an unset member now reads back as unknown rather than as
> a plausible number.
> 
> Found by code inspection. Tested on a QEMU CXL topology with two endpoints
> sharing a switch upstream link and HMAT generic-port coordinates for the
> host bridge, which the calculation requires to run at all: with temporary
> printk at the combine sites, the unpatched kernel summed 0xfefefefe, the
> CONFIG_INIT_STACK_ALL_PATTERN stack filler, into the latency members, while
> the patched kernel reports the CDAT latency values there.
> 
> Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Reviewed-by: Richard Cheng <icheng@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/cdat.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 5c9f07262513..df9261ba84e5 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -633,9 +633,9 @@ static int cxl_endpoint_gather_bandwidth(struct cxl_region *cxlr,
>  	struct cxl_port *endpoint = to_cxl_port(cxled->cxld.dev.parent);
>  	struct cxl_port *parent_port = to_cxl_port(endpoint->dev.parent);
>  	struct cxl_port *gp_port = to_cxl_port(parent_port->dev.parent);
> -	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX];
> -	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX];
> -	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX];
> +	struct access_coordinate pci_coord[ACCESS_COORDINATE_MAX] = { };
> +	struct access_coordinate sw_coord[ACCESS_COORDINATE_MAX] = { };
> +	struct access_coordinate ep_coord[ACCESS_COORDINATE_MAX] = { };
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> @@ -757,7 +757,7 @@ static struct xarray *cxl_switch_gather_bandwidth(struct cxl_region *cxlr,
>  {
>  	struct xarray *res_xa __free(free_perf_xa) =
>  		kzalloc_obj(*res_xa);
> -	struct access_coordinate coords[ACCESS_COORDINATE_MAX];
> +	struct access_coordinate coords[ACCESS_COORDINATE_MAX] = { };
>  	struct cxl_perf_ctx *ctx, *us_ctx;
>  	unsigned long index, us_index;
>  	int dev_count = 0;

[PATCH v4 2/2] cxl/port: Fix uninitialized coordinates reported for RCDs

[Guixin Liu]

cxl_endpoint_get_perf_coordinates() returns success for a Restricted CXL
Device without calculating coordinates, so the caller's output array is
left uninitialized. Callers treat it as valid and can expose the stack
residue as access coordinates.

Initialize the coordinates to zero before returning for an RCD. Zeroing in
the helper rather than at the caller keeps the @coord output contract the
exported function documents.

Reported by the Sashiko review bot.

Fixes: 5d211c709059 ("cxl: Fix cxl_endpoint_get_perf_coordinate() support for RCH")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Reviewed-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/port.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 625e4aa427db..fea43a92744c 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2394,8 +2394,10 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
 	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
 	 * since RCH does not support hotplug.
 	 */
-	if (cxlmd->cxlds->rcd)
+	if (cxlmd->cxlds->rcd) {
+		memset(coord, 0, sizeof(*coord) * ACCESS_COORDINATE_MAX);
 		return 0;
+	}
 
 	/*
 	 * Exit the loop when the parent port of the current iter port is cxl
-- 
2.43.7

Re: [PATCH v4 2/2] cxl/port: Fix uninitialized coordinates reported for RCDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Zeroing access coordinates for RCDs causes bandwidth calculation to bypass assignment, resulting in 0 bandwidth for QoS matching.

Pre-existing issues:
- [Critical] parent_port_is_cxl_root() causes a NULL pointer dereference for CXL devices directly connected to a Root Port.
--

commit 2dd3e928fa65a79b6a782bef4e7234009cf5df5e
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/port: Fix uninitialized coordinates reported for RCDs

This commit initializes the access_coordinate array to zero for Restricted CXL
Devices (RCDs) to prevent exposing uninitialized stack memory to callers.

> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 625e4aa427db0..fea43a92744cd 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2394,8 +2394,10 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
>  	 * since RCH does not support hotplug.
>  	 */
> -	if (cxlmd->cxlds->rcd)
> +	if (cxlmd->cxlds->rcd) {
> +		memset(coord, 0, sizeof(*coord) * ACCESS_COORDINATE_MAX);

[Severity: High]
Does zeroing the access coordinates break bandwidth calculations for RCDs?

The neutral/no-op value for CXL access coordinate bandwidth is UINT_MAX,
not 0. When cxl_coordinates_combine() is later called to merge these values,
__cxl_coordinates_combine() checks:

if (c1->write_bandwidth && c2->write_bandwidth)
        out->write_bandwidth = min(c1->write_bandwidth,
                                   c2->write_bandwidth);

Because this memset sets the bandwidth to 0, the condition evaluates to false,
bypassing the assignment. This leaves the output bandwidth unassigned (0),
which causes subsequent QoS class calculations to fail or misclassify the
device.

>  		return 0;
> +	}
>  
>  	/*
>  	 * Exit the loop when the parent port of the current iter port is cxl
>  	 * root. The iterative loop starts at the endpoint and gathers the
>  	 * latency of the CXL link from the current device/port to the connected
>  	 * downstream port each iteration.
>  	 */
>  	do {
>  		dport = iter->parent_dport;
>  		iter = to_cxl_port(iter->dev.parent);
>  		is_cxl_root = parent_port_is_cxl_root(iter);

[Severity: Critical]
This is a pre-existing issue, but does parent_port_is_cxl_root() cause a NULL
pointer dereference for CXL devices directly connected to a Root Port?

When the endpoint is directly attached to a CXL Root Port, the loop advances
iter to the CXL Root port itself. Passing iter to parent_port_is_cxl_root()
evaluates:

is_cxl_root(to_cxl_port(port->dev.parent))

Since iter is already the CXL Root port, its dev.parent is a platform device
(such as an ACPI device), not a cxl_port. to_cxl_port() fails the type check,
triggers a dev_WARN_ONCE, and returns NULL.

is_cxl_root() then unconditionally dereferences the NULL pointer in cxl.h:

return port->uport_dev == port->dev.parent;

This results in a kernel panic on standard boot and probe paths for common
topologies.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831092216.540644-1-kanie@linux.alibaba.com?part=2