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

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

[Guixin Liu]

Two related fixes for stack access_coordinate arrays that are read before
anything writes them, both reachable on real topologies and both ending up
in userspace through the access coordinate sysfs attributes.

Patch 1 is the v2 cdat fix [1] with one more site covered: the coords array
in cxl_switch_gather_bandwidth() has the same defect as the three arrays in
cxl_endpoint_gather_bandwidth().

Patch 2 is new. cxl_endpoint_get_perf_coordinates() returns 0 for an RCD
without writing the caller's array at all, so cxl_port_perf_data_calculate()
combines stack residue with the values parsed from the RCD's CDAT.

Both were pointed out by the Sashiko review bot on the v2 posting of patch 1.
The third finding in that review - cxl_coordinates_combine() aliasing its
output onto an input and leaving a member alone when an input bandwidth is
zero - is the intended "not reported" behaviour and is not addressed here.

[1] https://lore.kernel.org/linux-cxl/20260812060912.54932-1-kanie@linux.alibaba.com/

v2->v3:
- also zero initialize coords in cxl_switch_gather_bandwidth(), same defect
  in the sibling gather path (Sashiko bot)
- new patch 2: clear the output coordinates on the RCD path of
  cxl_endpoint_get_perf_coordinates(), which currently returns success
  without writing them (Sashiko bot)
- drop "endpoint" from the patch 1 subject, it now covers both gather paths

v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
  the code change (Alison Schofield)

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 v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[Guixin Liu]

cxl_endpoint_gather_bandwidth() declares three access_coordinate arrays on
the stack - pci_coord, sw_coord and ep_coord - without initializing them,
and relies on its helpers to fill every member. None of them does.
cxl_pci_get_bandwidth() and cxl_port_get_switch_dport_bandwidth() assign
only read_bandwidth and write_bandwidth, and __cxl_coordinates_combine()
assigns an output bandwidth only when both input bandwidths are non-zero.
Every member a producer declines to set is read back as whatever was on the
stack. cxl_switch_gather_bandwidth() has the same defect in its coords
array, which it hands to cxl_pci_get_bandwidth() and then to
cxl_coordinates_combine().

Both cases occur on real topologies. The latency members are never written
by any of those producers, yet __cxl_coordinates_combine() sums them
unconditionally, so the latency reads are undefined on every call. And a
device whose CDAT DSLBIS reports no bandwidth for an access class leaves
perf->cdat_coord zero for that class, which is exactly the condition that
makes __cxl_coordinates_combine() skip the bandwidth assignment and leave
the ep_coord entry untouched.

The ep_coord case escapes the function: cxl_bandwidth_add() accumulates it
into the per-upstream-port aggregate that is published through the region's
access coordinate sysfs attributes, so stack contents are reported to
userspace as a bandwidth figure. The latency sums are discarded by
cxl_bandwidth_add() rather than published, but they are still computed from
uninitialized memory.

Zero initialize the four arrays. Zero is already the value this code uses
for "not reported" - both the __cxl_coordinates_combine() guard and
coordinates_valid() test for it - so a member no producer sets now reads
back as unknown rather than as a plausible number.

Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.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 v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[sashiko-bot]

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

Pre-existing issues:
- [High] Zero-initializing `ep_coord` codifies a logic error where valid PCI bandwidth is discarded if CDAT bandwidth is unreported (0), contradicting subsystem conventions.
- [High] The cxl_port_perf_data_calculate() function declares an uninitialized access_coordinate array on the stack, which is leaked to userspace via sysfs for Restricted CXL Devices (RCDs).
--

--- Patch [1]: [PATCH v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering ---
commit ecbce27f92f7e67349081889c3ca38eb48761104
Author: Guixin Liu <kanie@linux.alibaba.com>
Subject: cxl/cdat: Fix uninitialized stack use in bandwidth gathering

This commit fixes uninitialized stack usage in cxl_endpoint_gather_bandwidth()
and cxl_switch_gather_bandwidth() by zero-initializing the access_coordinate
arrays. It prevents uninitialized kernel stack data from being exposed as
bandwidth and latency figures.

> 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
> @@ -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] = { };

[Severity: High]
This isn't a bug introduced by this patch, but does zero-initializing
ep_coord codify a logic error where valid PCI bandwidth is discarded?

If an endpoint CDAT does not report bandwidth, the bandwidth fields in
perf->cdat_coord will be 0. When cxl_coordinates_combine() is called later
in cxl_endpoint_gather_bandwidth():

	rc = cxl_pci_get_bandwidth(pdev, pci_coord);
	if (rc < 0)
		return rc;

	/*
	 * Min of upstream link bandwidth and Endpoint CDAT bandwidth from
	 * DSLBIS.
	 */
	cxl_coordinates_combine(ep_coord, pci_coord, perf->cdat_coord);

The __cxl_coordinates_combine() helper requires both inputs to be non-zero
to assign the output:

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

Because the bandwidth in perf->cdat_coord is 0, ep_coord remains untouched
(which is now 0). This completely discards the valid upstream link
bandwidth in pci_coord, resulting in an erroneous region aggregate
bandwidth of 0.

[Severity: High]
This is a pre-existing issue, but while fixing uninitialized stack
variables, did you notice a similar leak in cxl_port_perf_data_calculate()
in drivers/cxl/core/cdat.c?

It declares an uninitialized array:

	struct access_coordinate ep_c[ACCESS_COORDINATE_MAX];
	...
	rc = cxl_endpoint_get_perf_coordinates(port, ep_c);

If the device is a Restricted CXL Device (RCD),
cxl_endpoint_get_perf_coordinates() in drivers/cxl/core/port.c returns 0
early without initializing the array:

	/*
	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
	 * since RCH does not support hotplug.
	 */
	if (cxlmd->cxlds->rcd)
		return 0;

cxl_port_perf_data_calculate() then passes this uninitialized array to
cxl_coordinates_combine(), which can leak kernel stack memory to userspace
via world-readable sysfs region attributes.

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

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

[Guixin Liu]

在 2026/8/12 16:44, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Zero-initializing `ep_coord` codifies a logic error where valid PCI bandwidth is discarded if CDAT bandwidth is unreported (0), contradicting subsystem conventions.
> - [High] The cxl_port_perf_data_calculate() function declares an uninitialized access_coordinate array on the stack, which is leaked to userspace via sysfs for Restricted CXL Devices (RCDs).
> --
>
> --- Patch [1]: [PATCH v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering ---
> commit ecbce27f92f7e67349081889c3ca38eb48761104
> Author: Guixin Liu <kanie@linux.alibaba.com>
> Subject: cxl/cdat: Fix uninitialized stack use in bandwidth gathering
>
> This commit fixes uninitialized stack usage in cxl_endpoint_gather_bandwidth()
> and cxl_switch_gather_bandwidth() by zero-initializing the access_coordinate
> arrays. It prevents uninitialized kernel stack data from being exposed as
> bandwidth and latency figures.
>
>> 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
>> @@ -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] = { };
> [Severity: High]
> This isn't a bug introduced by this patch, but does zero-initializing
> ep_coord codify a logic error where valid PCI bandwidth is discarded?
>
> If an endpoint CDAT does not report bandwidth, the bandwidth fields in
> perf->cdat_coord will be 0. When cxl_coordinates_combine() is called later
> in cxl_endpoint_gather_bandwidth():
>
> 	rc = cxl_pci_get_bandwidth(pdev, pci_coord);
> 	if (rc < 0)
> 		return rc;
>
> 	/*
> 	 * Min of upstream link bandwidth and Endpoint CDAT bandwidth from
> 	 * DSLBIS.
> 	 */
> 	cxl_coordinates_combine(ep_coord, pci_coord, perf->cdat_coord);
>
> The __cxl_coordinates_combine() helper requires both inputs to be non-zero
> to assign the output:
>
> 	if (c1->write_bandwidth && c2->write_bandwidth)
> 		out->write_bandwidth = min(c1->write_bandwidth,
> 					   c2->write_bandwidth);
>
> Because the bandwidth in perf->cdat_coord is 0, ep_coord remains untouched
> (which is now 0). This completely discards the valid upstream link
> bandwidth in pci_coord, resulting in an erroneous region aggregate
> bandwidth of 0.
>
> [Severity: High]
> This is a pre-existing issue, but while fixing uninitialized stack
> variables, did you notice a similar leak in cxl_port_perf_data_calculate()
> in drivers/cxl/core/cdat.c?
>
> It declares an uninitialized array:
>
> 	struct access_coordinate ep_c[ACCESS_COORDINATE_MAX];
> 	...
> 	rc = cxl_endpoint_get_perf_coordinates(port, ep_c);
>
> If the device is a Restricted CXL Device (RCD),
> cxl_endpoint_get_perf_coordinates() in drivers/cxl/core/port.c returns 0
> early without initializing the array:
>
> 	/*
> 	 * Skip calculation for RCD. Expectation is HMAT already covers RCD case
> 	 * since RCH does not support hotplug.
> 	 */
> 	if (cxlmd->cxlds->rcd)
> 		return 0;
>
> cxl_port_perf_data_calculate() then passes this uninitialized array to
> cxl_coordinates_combine(), which can leak kernel stack memory to userspace
> via world-readable sysfs region attributes.
On the RCD leak in cxl_port_perf_data_calculate(): that is what patch 2 of
this series fixes. cxl_endpoint_get_perf_coordinates() now zeroes the
caller's array before the early return for RCDs, so ep_c[] is no longer read
before anything writes it. The fix is in the exported helper rather than in
the caller because the helper documents @coord as an output parameter, and
cxl_test discards its return value.

On ep_coord being discarded when the CDAT reports no bandwidth: the guard
that skips the assignment, "if (c1->write_bandwidth && c2->write_bandwidth)"
in __cxl_coordinates_combine(), is pre-existing and untouched by this patch.
What the patch changes is only what remains in the output when that guard
skips: stack residue before, zero now. A zero bandwidth is how that helper
already spells "not reported" - that is why the guard exists, since min()
against an unknown value is meaningless. Making the combine step fall back
to one input instead would change the meaning of the helper for all of its
callers, which is a separate discussion from not reading uninitialized
stack. The cover letter says the same about the aliasing question raised on
v2.

Best Regards,
Guixin Liu

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

[Alison Schofield]

On Wed, Aug 12, 2026 at 04:30:34PM +0800, Guixin Liu wrote:
> cxl_endpoint_gather_bandwidth() declares three access_coordinate arrays on
> the stack - pci_coord, sw_coord and ep_coord - without initializing them,
> and relies on its helpers to fill every member. None of them does.
> cxl_pci_get_bandwidth() and cxl_port_get_switch_dport_bandwidth() assign
> only read_bandwidth and write_bandwidth, and __cxl_coordinates_combine()
> assigns an output bandwidth only when both input bandwidths are non-zero.
> Every member a producer declines to set is read back as whatever was on the
> stack. cxl_switch_gather_bandwidth() has the same defect in its coords
> array, which it hands to cxl_pci_get_bandwidth() and then to
> cxl_coordinates_combine().
> 
> Both cases occur on real topologies. The latency members are never written
> by any of those producers, yet __cxl_coordinates_combine() sums them
> unconditionally, so the latency reads are undefined on every call. And a
> device whose CDAT DSLBIS reports no bandwidth for an access class leaves
> perf->cdat_coord zero for that class, which is exactly the condition that
> makes __cxl_coordinates_combine() skip the bandwidth assignment and leave
> the ep_coord entry untouched.
> 
> The ep_coord case escapes the function: cxl_bandwidth_add() accumulates it
> into the per-upstream-port aggregate that is published through the region's
> access coordinate sysfs attributes, so stack contents are reported to
> userspace as a bandwidth figure. The latency sums are discarded by
> cxl_bandwidth_add() rather than published, but they are still computed from
> uninitialized memory.
> 
> Zero initialize the four arrays. Zero is already the value this code uses
> for "not reported" - both the __cxl_coordinates_combine() guard and
> coordinates_valid() test for it - so a member no producer sets now reads
> back as unknown rather than as a plausible number.


Hi Guixin Liu,

Please see my comments on the einj_inject patch about commit message
clarity and level of detail. The same issue applies here.
https://lore.kernel.org/linux-cxl/apDXSzSZVcjQ027J@aschofie-mobl2.lan/

For this patch, I think the essential story is:

Background:
cxl_endpoint_get_perf_coordinates() returns success for RCDs without
calculating coordinates.

Issue:
The output array is left uninitialized on that path.

Impact:
Callers can consume and expose garbage access-coordinate values.

Resolution:
Initialize the coordinates to zero before returning for an RCD.

That is enough to explain why the patch is needed. I do not need the
full path through CDAT parsing, __cxl_coordinates_combine(), QoS class
selection, cxl_dpa_perf, and sysfs to understand the bug.

Also, as with the other patch, please say how this was found and how it
was tested.

-- Alison


> 
> Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.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 v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[Alison Schofield]

On Thu, Aug 27, 2026 at 05:49:47PM -0700, Alison Schofield wrote:
> On Wed, Aug 12, 2026 at 04:30:34PM +0800, Guixin Liu wrote:
> > cxl_endpoint_gather_bandwidth() declares three access_coordinate arrays on
> > the stack - pci_coord, sw_coord and ep_coord - without initializing them,
> > and relies on its helpers to fill every member. None of them does.
> > cxl_pci_get_bandwidth() and cxl_port_get_switch_dport_bandwidth() assign
> > only read_bandwidth and write_bandwidth, and __cxl_coordinates_combine()
> > assigns an output bandwidth only when both input bandwidths are non-zero.
> > Every member a producer declines to set is read back as whatever was on the
> > stack. cxl_switch_gather_bandwidth() has the same defect in its coords
> > array, which it hands to cxl_pci_get_bandwidth() and then to
> > cxl_coordinates_combine().
> > 
> > Both cases occur on real topologies. The latency members are never written
> > by any of those producers, yet __cxl_coordinates_combine() sums them
> > unconditionally, so the latency reads are undefined on every call. And a
> > device whose CDAT DSLBIS reports no bandwidth for an access class leaves
> > perf->cdat_coord zero for that class, which is exactly the condition that
> > makes __cxl_coordinates_combine() skip the bandwidth assignment and leave
> > the ep_coord entry untouched.
> > 
> > The ep_coord case escapes the function: cxl_bandwidth_add() accumulates it
> > into the per-upstream-port aggregate that is published through the region's
> > access coordinate sysfs attributes, so stack contents are reported to
> > userspace as a bandwidth figure. The latency sums are discarded by
> > cxl_bandwidth_add() rather than published, but they are still computed from
> > uninitialized memory.
> > 
> > Zero initialize the four arrays. Zero is already the value this code uses
> > for "not reported" - both the __cxl_coordinates_combine() guard and
> > coordinates_valid() test for it - so a member no producer sets now reads
> > back as unknown rather than as a plausible number.
> 
> 
> Hi Guixin Liu,
> 
> Please see my comments on the einj_inject patch about commit message
> clarity and level of detail. The same issue applies here.
> https://lore.kernel.org/linux-cxl/apDXSzSZVcjQ027J@aschofie-mobl2.lan/
> 
> For this patch, I think the essential story is:

Oops, I just criss-crossed your 2 patches. This goes with Patch 2/2.
Similar feedback on both patches.
Thanks,

-- Alison




> 
> Background:
> cxl_endpoint_get_perf_coordinates() returns success for RCDs without
> calculating coordinates.
> 
> Issue:
> The output array is left uninitialized on that path.
> 
> Impact:
> Callers can consume and expose garbage access-coordinate values.
> 
> Resolution:
> Initialize the coordinates to zero before returning for an RCD.
> 
> That is enough to explain why the patch is needed. I do not need the
> full path through CDAT parsing, __cxl_coordinates_combine(), QoS class
> selection, cxl_dpa_perf, and sysfs to understand the bug.
> 
> Also, as with the other patch, please say how this was found and how it
> was tested.
> 
> -- Alison
> 
> 
> > 
> > Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
> > Signed-off-by: Guixin Liu <kanie@linux.alibaba.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 v3 1/2] cxl/cdat: Fix uninitialized stack use in bandwidth gathering

[Guixin Liu]

在 2026/8/28 08:49, Alison Schofield 写道:
> On Wed, Aug 12, 2026 at 04:30:34PM +0800, Guixin Liu wrote:
>> cxl_endpoint_gather_bandwidth() declares three access_coordinate arrays on
>> the stack - pci_coord, sw_coord and ep_coord - without initializing them,
>> and relies on its helpers to fill every member. None of them does.
>> cxl_pci_get_bandwidth() and cxl_port_get_switch_dport_bandwidth() assign
>> only read_bandwidth and write_bandwidth, and __cxl_coordinates_combine()
>> assigns an output bandwidth only when both input bandwidths are non-zero.
>> Every member a producer declines to set is read back as whatever was on the
>> stack. cxl_switch_gather_bandwidth() has the same defect in its coords
>> array, which it hands to cxl_pci_get_bandwidth() and then to
>> cxl_coordinates_combine().
>>
>> Both cases occur on real topologies. The latency members are never written
>> by any of those producers, yet __cxl_coordinates_combine() sums them
>> unconditionally, so the latency reads are undefined on every call. And a
>> device whose CDAT DSLBIS reports no bandwidth for an access class leaves
>> perf->cdat_coord zero for that class, which is exactly the condition that
>> makes __cxl_coordinates_combine() skip the bandwidth assignment and leave
>> the ep_coord entry untouched.
>>
>> The ep_coord case escapes the function: cxl_bandwidth_add() accumulates it
>> into the per-upstream-port aggregate that is published through the region's
>> access coordinate sysfs attributes, so stack contents are reported to
>> userspace as a bandwidth figure. The latency sums are discarded by
>> cxl_bandwidth_add() rather than published, but they are still computed from
>> uninitialized memory.
>>
>> Zero initialize the four arrays. Zero is already the value this code uses
>> for "not reported" - both the __cxl_coordinates_combine() guard and
>> coordinates_valid() test for it - so a member no producer sets now reads
>> back as unknown rather than as a plausible number.
>
> Hi Guixin Liu,
>
> Please see my comments on the einj_inject patch about commit message
> clarity and level of detail. The same issue applies here.
> https://lore.kernel.org/linux-cxl/apDXSzSZVcjQ027J@aschofie-mobl2.lan/
>
> For this patch, I think the essential story is:
>
> Background:
> cxl_endpoint_get_perf_coordinates() returns success for RCDs without
> calculating coordinates.
>
> Issue:
> The output array is left uninitialized on that path.
>
> Impact:
> Callers can consume and expose garbage access-coordinate values.
>
> Resolution:
> Initialize the coordinates to zero before returning for an RCD.
>
> That is enough to explain why the patch is needed. I do not need the
> full path through CDAT parsing, __cxl_coordinates_combine(), QoS class
> selection, cxl_dpa_perf, and sysfs to understand the bug.
>
> Also, as with the other patch, please say how this was found and how it
> was tested.
>
> -- Alison
Sure, I will cut the commit and explain how I found it and how it was 
tested in v4.

Best Regards,
Guixin Liu
>
>
>> Fixes: a5ab0de0ebaa ("cxl: Calculate region bandwidth of targets with shared upstream link")
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.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
>>

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

[Guixin Liu]

cxl_endpoint_get_perf_coordinates() walks the CXL path and copies the
accumulated numbers into the caller's array with set_access_coordinates()
just before it returns. For a Restricted CXL Device it never gets that far:
RCH topologies are covered by HMAT and cannot be hotplugged, so the walk is
skipped and the function returns 0 with the output array untouched. Every
caller is told the coordinates are valid while nothing was written to them.

Its only consumer, cxl_port_perf_data_calculate(), keeps the array on the
stack and does not pre-zero it, and endpoint CDAT parsing is not gated on
the topology: cxl_endpoint_port_probe() calls read_cdat_data() and
cxl_endpoint_parse_cdat() for every endpoint port, RCD included. So an RCD
that exposes a CDAT combines stack residue with the values from its DSMAS
entries.

The residue does not stay in the driver. __cxl_coordinates_combine() sums
the latencies unconditionally and takes the minimum of the bandwidths when
both are non-zero, so it lands in dent->coord, which selects a QoS class
through cxl_root->ops.qos_class() and is copied into the memdev's
cxl_dpa_perf, where cxl_memdev_update_perf() publishes it through the
memdev's access coordinate sysfs attributes.

Clear the output on the RCD path so the function honours its contract for
every caller. Zeroing at the single caller instead would leave the exported
helper free to report success without writing anything, which the next
caller would have to know about; and zero is the value this code already
uses for "not reported", so an RCD now reads back as unknown rather than as
plausible numbers.

Fixes: 5d211c709059 ("cxl: Fix cxl_endpoint_get_perf_coordinate() support for RCH")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.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 v3 0/2] cxl: Fix uninitialized access coordinates

[Richard Cheng]

On Wed, Aug 12, 2026 at 04:30:33PM +0800, Guixin Liu wrote:
> Two related fixes for stack access_coordinate arrays that are read before
> anything writes them, both reachable on real topologies and both ending up
> in userspace through the access coordinate sysfs attributes.
> 
> Patch 1 is the v2 cdat fix [1] with one more site covered: the coords array
> in cxl_switch_gather_bandwidth() has the same defect as the three arrays in
> cxl_endpoint_gather_bandwidth().
> 
> Patch 2 is new. cxl_endpoint_get_perf_coordinates() returns 0 for an RCD
> without writing the caller's array at all, so cxl_port_perf_data_calculate()
> combines stack residue with the values parsed from the RCD's CDAT.
> 
> Both were pointed out by the Sashiko review bot on the v2 posting of patch 1.
> The third finding in that review - cxl_coordinates_combine() aliasing its
> output onto an input and leaving a member alone when an input bandwidth is
> zero - is the intended "not reported" behaviour and is not addressed here.
>

Hi Guixin,

Thanks for this, both changes LGTM.

Reviewed-by: Richard Cheng <icheng@nvidia.com>

Best regards,
Richard Cheng.

> [1] https://lore.kernel.org/linux-cxl/20260812060912.54932-1-kanie@linux.alibaba.com/
> 
> v2->v3:
> - also zero initialize coords in cxl_switch_gather_bandwidth(), same defect
>   in the sibling gather path (Sashiko bot)
> - new patch 2: clear the output coordinates on the RCD path of
>   cxl_endpoint_get_perf_coordinates(), which currently returns success
>   without writing them (Sashiko bot)
> - drop "endpoint" from the patch 1 subject, it now covers both gather paths
> 
> v1->v2:
> - rebase onto cxl/next
> - rewrite the commit message to describe the behaviour rather than narrate
>   the code change (Alison Schofield)
> 
> 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
> 
>