[PATCH v2] cxl/hdm: Fix out of bounds read of the decoder target list

Guixin Liu <[email protected]>
Newsgroups org.kernel.vger.linux-cxl
Message-ID <[email protected]>
init_hdm_decoder() reads the HDM Decoder Target List register of a switch
or host bridge decoder into an 8 byte on-stack image and copies
cxld->interleave_ways bytes of it into cxld->target_map, with nothing
bounding the count against the 8 target port IDs the register can hold.
cxl_port_setup_targets(), which programs the same field from the region
side, refuses the configuration up front with 'iw > 8 || iw > nr_targets';
the enumeration path has no equivalent gate.

Values above 8 are legal hardware encodings, not corruption.
interleave_ways comes from the 4-bit Interleave Ways field of the decoder
control register through eiw_to_ways(), which returns 16 for eiw 4 and 12
for eiw 10, and the driver accepts whatever firmware left programmed there
before it enumerated the decoder.

The copy loop therefore reads up to 8 bytes past the union, and the stack
residue is stored into target_map as target port IDs. Those IDs are matched
against the port's dports by find_dport() when the decoder's targets are
populated, so a byte that happens to match an unrelated dport's port_id
installs that dport into cxlsd->target[].

Reject the decoder, which is how the eiw_to_ways() and eig_to_granularity()
failures in the same function are already handled. A decoder whose target
list register cannot describe its own interleave is not a configuration a
region can be attached to.

Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
Signed-off-by: Guixin Liu <[email protected]>
---
This was patch 5/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/hdm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..9d49b48a4456 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1084,6 +1084,18 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 		cxld->interleave_ways, cxld->interleave_granularity);
 
 	if (!cxled) {
+		/*
+		 * The Target List register only holds
+		 * ARRAY_SIZE(target_list.target_id) entries, so a switch
+		 * decoder cannot interleave across more ports than that.
+		 */
+		if (cxld->interleave_ways > ARRAY_SIZE(target_list.target_id)) {
+			dev_warn(&port->dev,
+				 "decoder%d.%d: Interleave ways: %d exceeds target list size\n",
+				 port->id, cxld->id, cxld->interleave_ways);
+			return -ENXIO;
+		}
+
 		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
 		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
 		target_list.value = (hi << 32) + lo;

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
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.