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

[sashiko-bot@kernel.org]

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