[PATCH v2] cxl/cdat: Fix uninitialized stack use in endpoint 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.
Both cases occur on real topologies. The latency members of pci_coord and
sw_coord are never written, 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 three 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]>
---
This was patch 6/8 of the "cxl: Assorted fixes" series [1]. Per review
feedback that series is not being reworked as a whole; the fixes are resent
individually instead. Patches 1, 2 and 7 of the series are dropped, as those
issues are already fixed in cxl/next.
v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
the code change (Alison Schofield)
[1] https://lore.kernel.org/linux-cxl/[email protected]/
drivers/cxl/core/cdat.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index 5c9f07262513..3c6a1537f89b 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);
base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
--
2.43.7