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

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 <[email protected]>
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/[email protected]?part=1