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

[Alison Schofield <alison.schofield@intel.com>]

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
>