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

Guixin Liu <[email protected]>
Newsgroups org.kernel.vger.linux-cxl
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.