[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