* [PATCH v5 0/4] Harden HDM decoder enumeration
@ 2026-08-08 0:05 Alison Schofield
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:05 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Li Ming
Cc: linux-cxl, Srirangan Madhavan
Changes in v5:
Drop P2 of v4 as it changed what user sees in iw and ig attributes (Davidlohr)
Renumbering: v4 P3 and P4 are now P2 and P3
P1: Validate the target count once per port, not per decoder (Jonathan)
P1: Move the zero-target reject back to cxl_switch_decoder_init() (Ming)
P1: Drop the ways reset for uncommitted decoders (Jonathan, Davidlohr)
P1: Declare cxlsd on one line (Jonathan)
P1: Reword the CXL_HDM_DECODER0_TL_TARGETS comment (Jonathan)
P2: Build the target IDs with put_unaligned_le32() (Jonathan)
P2: Dropped all the tags on endian-ness patch
P3: Commit log s/none committed/uncommitted/ (DaveJ)
P4: New - bound switch decoder target array walks by nr_targets
Changes in v4:
A new patch was inserted as Patch2, so now set is 4 patches.
Sashiko is likely to complain in Patch 1 for something fixed in
Patches 2 and 3, but reordering does not makes sense.
P2: Only interpret interleave settings on committed decoders (Ming)
This is the newly inserted patch
P4: Register reset w devm_add_action (Ming)
P4: Fix up a kernel-doc comment issue (lkp)
Changes in v3:
P1: Move zero-target reject to init_hdm_decoder() (RichardC)
P3: Reset commit_end via a devres action after decoder teardown (RichardC)
Changes in v2:
P1: Handle uncommitted decoders (Sashiko)
P1: Reject switch decoders with zero targets (Sashiko)
P1: Add in code comment clarifying target_count (Sashiko)
P1: Update commit log
P3: Reset commit_end on the failure path only (Sashiko)
P3: Update commit log
P2: No
This set started as Sashiko reported cleanups. The patch added at v4 on
Li Ming's review is dropped in v5. Davidlohr pointed out that it changes
what interleave_ways and interleave_granularity report for an uncommitted
decoder, and those are documented ABI. That is also the answer to Ming's
v3 question that prompted the patch - enumeration decodes those fields
because userspace reads them, not because the driver needs them.
Only the first patch includes a link to the original Sashiko complaint,
since the others did not originally appear on Lore for me to add public
links.
Srirangan: I believe some of these patches address pre-existing issues
Sashiko posted in reply to your reset series, but frankly, with the
renumbering here, and the respin of your series, I no longer have
the exact patch to patch correlation.
Alison Schofield (4):
cxl/hdm: Reject switch decoder interleave ways that overflow targets
cxl/hdm: Make switch decoder target parsing endian-safe
cxl/hdm: Restore commit_end when decoder enumeration fails
cxl/port: Bound switch decoder target array access by nr_targets
drivers/cxl/core/hdm.c | 62 +++++++++++++++++++++++++++++++++--------
drivers/cxl/core/port.c | 27 ++++++++++++------
drivers/cxl/cxl.h | 2 ++
3 files changed, 72 insertions(+), 19 deletions(-)
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
--
2.37.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets
2026-08-08 0:05 [PATCH v5 0/4] Harden HDM decoder enumeration Alison Schofield
@ 2026-08-08 0:05 ` Alison Schofield
2026-08-08 0:17 ` sashiko-bot
2026-08-08 5:50 ` Li Ming
2026-08-08 0:05 ` [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe Alison Schofield
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:05 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Li Ming
Cc: linux-cxl, Srirangan Madhavan, Richard Cheng
Switch decoder enumeration validates that the interleave ways encoding
is legal, but not that the resulting number of ways fits the available
targets. This can overrun the target array (target_list.target_id[])
during enumeration of 12 or 16 way interleaves.
Bound the target count where it is established rather than at each use.
Reject an HDM capability whose reported target count is zero or exceeds
the target list register capacity, and reject a zero target switch
decoder allocation in cxl_switch_decoder_init(). Enumeration is then
left with a single check: reject a decoder whose interleave ways exceed
the targets it has.
Add a clarifying comment that target_count is a direct count, not
0-based like decoder_count.
Link: https://sashiko.dev/#/patchset/20260711180755.1779002-1-alison.schofield@intel.com?part=1
Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
Tested-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Richard Cheng <icheng@nvidia.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
drivers/cxl/core/hdm.c | 24 ++++++++++++++++++++++++
drivers/cxl/core/port.c | 2 +-
drivers/cxl/cxl.h | 2 ++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..c394b3d54d36 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -76,6 +76,8 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
+
+ /* target_count is a direct count (1h..8h), not 0-based like decoder_count */
cxlhdm->target_count =
FIELD_GET(CXL_HDM_DECODER_TARGET_COUNT_MASK, hdm_cap);
if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_11_8, hdm_cap))
@@ -1084,6 +1086,16 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
cxld->interleave_ways, cxld->interleave_granularity);
if (!cxled) {
+ struct cxl_switch_decoder *cxlsd = to_cxl_switch_decoder(&cxld->dev);
+
+ if (cxld->interleave_ways > cxlsd->nr_targets) {
+ dev_err(&port->dev,
+ "decoder%d.%d: interleave ways: %d exceeds targets: %d\n",
+ port->id, cxld->id, cxld->interleave_ways,
+ cxlsd->nr_targets);
+ 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;
@@ -1161,6 +1173,18 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
int i;
u64 dpa_base = 0;
+ /*
+ * Per CXL 4.0 8.2.4.20.1 Target Count is the number of target ports
+ * per decoder, max 8. Endpoint decoders have no targets.
+ */
+ if (!is_cxl_endpoint(port) &&
+ (cxlhdm->target_count < 1 ||
+ cxlhdm->target_count > CXL_HDM_DECODER0_TL_TARGETS)) {
+ dev_err(&port->dev, "Invalid decoder target count: %u\n",
+ cxlhdm->target_count);
+ return -ENXIO;
+ }
+
cxl_settle_decoders(cxlhdm);
for (i = 0; i < cxlhdm->decoder_count; i++) {
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1215ee4f4035..28eccfdd75b8 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1979,7 +1979,7 @@ static int cxl_switch_decoder_init(struct cxl_port *port,
struct cxl_switch_decoder *cxlsd,
int nr_targets)
{
- if (nr_targets > CXL_DECODER_MAX_INTERLEAVE)
+ if (nr_targets < 1 || nr_targets > CXL_DECODER_MAX_INTERLEAVE)
return -EINVAL;
cxlsd->nr_targets = nr_targets;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index c0e5308e4d1b..a1f32d39b24c 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -67,6 +67,8 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
#define CXL_HDM_DECODER0_CTRL_HOSTONLY BIT(12)
#define CXL_HDM_DECODER0_TL_LOW(i) (0x20 * (i) + 0x24)
#define CXL_HDM_DECODER0_TL_HIGH(i) (0x20 * (i) + 0x28)
+/* Target list capacity, i.e. max interleave ways for a non-endpoint decoder */
+#define CXL_HDM_DECODER0_TL_TARGETS 8
#define CXL_HDM_DECODER0_SKIP_LOW(i) CXL_HDM_DECODER0_TL_LOW(i)
#define CXL_HDM_DECODER0_SKIP_HIGH(i) CXL_HDM_DECODER0_TL_HIGH(i)
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe
2026-08-08 0:05 [PATCH v5 0/4] Harden HDM decoder enumeration Alison Schofield
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
@ 2026-08-08 0:05 ` Alison Schofield
2026-08-08 5:51 ` Li Ming
2026-08-08 0:05 ` [PATCH v5 3/4] cxl/hdm: Restore commit_end when decoder enumeration fails Alison Schofield
2026-08-08 0:05 ` [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets Alison Schofield
3 siblings, 1 reply; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:05 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Li Ming
Cc: linux-cxl, Srirangan Madhavan
Switch decoder target IDs are stored one per byte, starting with
target 0 in the least significant byte. The current code accesses
those bytes in memory order, which only matches register order on
little-endian hosts.
On big-endian hosts, the target IDs are reversed and the decoder is
associated with the wrong downstream ports.
Place the register values in a byte array in little-endian order and
index the target IDs from there, so the target mapping is independent
of host endianness.
Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
drivers/cxl/core/hdm.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index c394b3d54d36..fc22d515b638 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
#include <linux/seq_file.h>
+#include <linux/unaligned.h>
#include <linux/device.h>
#include <linux/delay.h>
@@ -975,15 +976,12 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
u64 *dpa_base, struct cxl_endpoint_dvsec_info *info)
{
struct cxl_endpoint_decoder *cxled = NULL;
+ u8 target_id[CXL_HDM_DECODER0_TL_TARGETS];
u64 size, base, skip, dpa_size, lo, hi;
bool committed;
u32 remainder;
int i, rc;
u32 ctrl;
- union {
- u64 value;
- unsigned char target_id[8];
- } target_list;
if (should_emulate_decoders(info))
return cxl_setup_hdm_decoder_from_dvsec(port, cxld, dpa_base,
@@ -1096,11 +1094,12 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
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;
+ put_unaligned_le32(readl(hdm + CXL_HDM_DECODER0_TL_LOW(which)),
+ &target_id[0]);
+ put_unaligned_le32(readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which)),
+ &target_id[4]);
for (i = 0; i < cxld->interleave_ways; i++)
- cxld->target_map[i] = target_list.target_id[i];
+ cxld->target_map[i] = target_id[i];
return 0;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/4] cxl/hdm: Restore commit_end when decoder enumeration fails
2026-08-08 0:05 [PATCH v5 0/4] Harden HDM decoder enumeration Alison Schofield
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
2026-08-08 0:05 ` [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe Alison Schofield
@ 2026-08-08 0:05 ` Alison Schofield
2026-08-08 0:05 ` [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets Alison Schofield
3 siblings, 0 replies; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:05 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Li Ming
Cc: linux-cxl, Srirangan Madhavan, Jonathan Cameron, Richard Cheng
commit_end tracks the highest committed decoder on a port. It gets
advanced before decoder enumeration is complete, so a later failure
leaves it pointing at a decoder that was never added. The next probe
then rejects decoder0 as out of order and enumeration fails.
Reset commit_end to the uncommitted baseline once the decoders are
torn down, so a subsequent probe rebuilds it from scratch. Register
the reset before the decoders are added so that on unwind it runs
after every decoder has been unregistered. This prevents commit_end
from dropping below the highest committed decoder still registered and
exposing an inconsistent value to a concurrent reader.
Protect the commit_end advance in the DVSEC emulation path against
concurrent readers, matching the register-programmed path.
Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
Fixes: b777e9bec960 ("cxl/hdm: Emulate HDM decoder from DVSEC range registers")
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Richard Cheng <icheng@nvidia.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
drivers/cxl/core/hdm.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index fc22d515b638..cb24901dd289 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -956,7 +956,8 @@ static int cxl_setup_hdm_decoder_from_dvsec(
* change the range registers at run time.
*/
cxld->flags |= CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK;
- port->commit_end = cxld->id;
+ scoped_guard(rwsem_write, &cxl_rwsem.region)
+ port->commit_end = cxld->id;
rc = devm_cxl_dpa_reserve(cxled, *dpa_base, len, 0);
if (rc) {
@@ -1159,6 +1160,14 @@ static void cxl_settle_decoders(struct cxl_hdm *cxlhdm)
msleep(20);
}
+static void cxl_reset_commit_end(void *data)
+{
+ struct cxl_port *port = data;
+
+ guard(rwsem_write)(&cxl_rwsem.region);
+ port->commit_end = -1;
+}
+
/**
* devm_cxl_enumerate_decoders - add decoder objects per HDM register set
* @cxlhdm: Structure to populate with HDM capabilities
@@ -1169,8 +1178,8 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
{
void __iomem *hdm = cxlhdm->regs.hdm_decoder;
struct cxl_port *port = cxlhdm->port;
- int i;
u64 dpa_base = 0;
+ int i, rc;
/*
* Per CXL 4.0 8.2.4.20.1 Target Count is the number of target ports
@@ -1186,8 +1195,16 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
cxl_settle_decoders(cxlhdm);
+ /*
+ * Reset commit_end after all decoders have been torn down so a
+ * subsequent probe rebuilds it from scratch.
+ */
+ rc = devm_add_action(&port->dev, cxl_reset_commit_end, port);
+ if (rc)
+ return rc;
+
for (i = 0; i < cxlhdm->decoder_count; i++) {
- int rc, target_count = cxlhdm->target_count;
+ int target_count = cxlhdm->target_count;
struct cxl_decoder *cxld;
if (is_cxl_endpoint(port)) {
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets
2026-08-08 0:05 [PATCH v5 0/4] Harden HDM decoder enumeration Alison Schofield
` (2 preceding siblings ...)
2026-08-08 0:05 ` [PATCH v5 3/4] cxl/hdm: Restore commit_end when decoder enumeration fails Alison Schofield
@ 2026-08-08 0:05 ` Alison Schofield
2026-08-08 5:52 ` Li Ming
3 siblings, 1 reply; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:05 UTC (permalink / raw)
To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
Vishal Verma, Ira Weiny, Li Ming
Cc: linux-cxl, Srirangan Madhavan
Switch decoder target lookups walk the target array using interleave
ways as the bound. The array is allocated with nr_targets entries, so a
decoder whose interleave ways exceeds nr_targets reads and writes past
the end of it. That happens during dport activation, during decoder
target population, and when userspace reads the target_list attribute.
Bounding by nr_targets alone is not correct either. Commit d6488fee6647
("cxl/port: Fix decoder initialization when nr_targets > interleave_ways")
moved target population to interleave ways precisely so that targets a
decoder does not use are left unpopulated. Neither field alone is a safe
bound, so add a helper that returns the minimum of the two and use it at
every walk of the target array.
Valid configurations keep interleave ways within nr_targets, so this is
not a behavior change for them.
Fixes: 40ba17afdfab ("cxl/acpi: Introduce cxl_decoder objects")
Fixes: d6488fee6647 ("cxl/port: Fix decoder initialization when nr_targets > interleave_ways")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
drivers/cxl/core/port.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 28eccfdd75b8..757b48584ac2 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -147,20 +147,29 @@ static ssize_t target_type_show(struct device *dev,
}
static DEVICE_ATTR_RO(target_type);
+/*
+ * Interleave ways selects how many targets a decoder uses, but the target
+ * array is only nr_targets long. Bound array access by both.
+ */
+static int cxlsd_nr_used_targets(struct cxl_switch_decoder *cxlsd)
+{
+ return min(cxlsd->cxld.interleave_ways, cxlsd->nr_targets);
+}
+
static ssize_t emit_target_list(struct cxl_switch_decoder *cxlsd, char *buf)
{
- struct cxl_decoder *cxld = &cxlsd->cxld;
+ int nr_used = cxlsd_nr_used_targets(cxlsd);
ssize_t offset = 0;
int i, rc = 0;
- for (i = 0; i < cxld->interleave_ways; i++) {
+ for (i = 0; i < nr_used; i++) {
struct cxl_dport *dport = cxlsd->target[i];
struct cxl_dport *next = NULL;
if (!dport)
break;
- if (i + 1 < cxld->interleave_ways)
+ if (i + 1 < nr_used)
next = cxlsd->target[i + 1];
rc = sysfs_emit_at(buf, offset, "%d%s", dport->port_id,
next ? "," : "");
@@ -1603,7 +1612,7 @@ static int update_decoder_targets(struct device *dev, void *data)
struct cxl_dport *dport = data;
struct cxl_switch_decoder *cxlsd;
struct cxl_decoder *cxld;
- int i;
+ int i, nr_used;
if (!is_switch_decoder(dev))
return 0;
@@ -1611,8 +1620,9 @@ static int update_decoder_targets(struct device *dev, void *data)
cxlsd = to_cxl_switch_decoder(dev);
cxld = &cxlsd->cxld;
guard(rwsem_write)(&cxl_rwsem.region);
+ nr_used = cxlsd_nr_used_targets(cxlsd);
- for (i = 0; i < cxld->interleave_ways; i++) {
+ for (i = 0; i < nr_used; i++) {
if (cxld->target_map[i] == dport->port_id) {
cxlsd->target[i] = dport;
dev_dbg(dev, "dport%d found in target list, index %d\n",
@@ -1910,7 +1920,7 @@ static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
struct cxl_port *port)
{
struct cxl_decoder *cxld = &cxlsd->cxld;
- int i;
+ int i, nr_used;
device_lock_assert(&port->dev);
@@ -1918,7 +1928,8 @@ static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
return 0;
guard(rwsem_write)(&cxl_rwsem.region);
- for (i = 0; i < cxlsd->cxld.interleave_ways; i++) {
+ nr_used = cxlsd_nr_used_targets(cxlsd);
+ for (i = 0; i < nr_used; i++) {
struct cxl_dport *dport = find_dport(port, cxld->target_map[i]);
if (!dport) {
--
2.37.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
@ 2026-08-08 0:17 ` sashiko-bot
2026-08-08 0:36 ` Alison Schofield
2026-08-08 5:50 ` Li Ming
1 sibling, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-08-08 0:17 UTC (permalink / raw)
To: Alison Schofield; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect parsing of the target list register on Big-Endian architectures.
--
--- Patch [1]: [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets ---
commit f23bc6bdfaf477309897aefc0c8b1cee5f060c37
Author: Alison Schofield <alison.schofield@intel.com>
cxl/hdm: Reject switch decoder interleave ways that overflow targets
Switch decoder enumeration validates that the interleave ways encoding
is legal, but not that the resulting number of ways fits the available
targets. This can overrun the target array (target_list.target_id[])
during enumeration of 12 or 16 way interleaves.
Link: https://sashiko.dev/#/patchset/20260711180755.1779002-1-alison.schofield@intel.com?part=1
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..c394b3d54d362 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
[ ... ]
> @@ -1084,6 +1086,16 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> cxld->interleave_ways, cxld->interleave_granularity);
>
> if (!cxled) {
> + struct cxl_switch_decoder *cxlsd = to_cxl_switch_decoder(&cxld->dev);
> +
> + if (cxld->interleave_ways > cxlsd->nr_targets) {
> + dev_err(&port->dev,
> + "decoder%d.%d: interleave ways: %d exceeds targets: %d\n",
> + port->id, cxld->id, cxld->interleave_ways,
> + cxlsd->nr_targets);
> + 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;
[Severity: High]
This isn't a bug introduced by this patch, but does this logic in
init_hdm_decoder() parse the target list register incorrectly on Big-Endian
architectures?
Because hi and lo are in CPU endianness, on a Big-Endian system the shift
operation places the most significant bytes of hi at the lowest addresses of
the target_list.target_id array, effectively reversing the hardware target
IDs.
When the 64-bit target list is read out sequentially as an array of bytes,
could this result in physical memory addresses being routed to the wrong
target ports on Big-Endian systems?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786143520.git.alison.schofield@intel.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets
2026-08-08 0:17 ` sashiko-bot
@ 2026-08-08 0:36 ` Alison Schofield
0 siblings, 0 replies; 10+ messages in thread
From: Alison Schofield @ 2026-08-08 0:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-cxl
On Sat, Aug 08, 2026 at 12:17:32AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incorrect parsing of the target list register on Big-Endian architectures.
Fixed in patch 2 of this series.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
2026-08-08 0:17 ` sashiko-bot
@ 2026-08-08 5:50 ` Li Ming
1 sibling, 0 replies; 10+ messages in thread
From: Li Ming @ 2026-08-08 5:50 UTC (permalink / raw)
To: Alison Schofield
Cc: linux-cxl, Srirangan Madhavan, Richard Cheng, Davidlohr Bueso,
Jonathan Cameron, Dave Jiang, Vishal Verma, Ira Weiny
On 8/8/2026 8:05 AM, Alison Schofield wrote:
> Switch decoder enumeration validates that the interleave ways encoding
> is legal, but not that the resulting number of ways fits the available
> targets. This can overrun the target array (target_list.target_id[])
> during enumeration of 12 or 16 way interleaves.
>
> Bound the target count where it is established rather than at each use.
> Reject an HDM capability whose reported target count is zero or exceeds
> the target list register capacity, and reject a zero target switch
> decoder allocation in cxl_switch_decoder_init(). Enumeration is then
> left with a single check: reject a decoder whose interleave ways exceed
> the targets it has.
>
> Add a clarifying comment that target_count is a direct count, not
> 0-based like decoder_count.
>
> Link: https://sashiko.dev/#/patchset/20260711180755.1779002-1-alison.schofield@intel.com?part=1
> Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
> Tested-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe
2026-08-08 0:05 ` [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe Alison Schofield
@ 2026-08-08 5:51 ` Li Ming
0 siblings, 0 replies; 10+ messages in thread
From: Li Ming @ 2026-08-08 5:51 UTC (permalink / raw)
To: Alison Schofield
Cc: linux-cxl, Srirangan Madhavan, Davidlohr Bueso, Jonathan Cameron,
Dave Jiang, Vishal Verma, Ira Weiny
On 8/8/2026 8:05 AM, Alison Schofield wrote:
> Switch decoder target IDs are stored one per byte, starting with
> target 0 in the least significant byte. The current code accesses
> those bytes in memory order, which only matches register order on
> little-endian hosts.
>
> On big-endian hosts, the target IDs are reversed and the decoder is
> associated with the wrong downstream ports.
>
> Place the register values in a byte array in little-endian order and
> index the target IDs from there, so the target mapping is independent
> of host endianness.
>
> Fixes: d17d0540a0db ("cxl/core/hdm: Add CXL standard decoder enumeration to the core")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets
2026-08-08 0:05 ` [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets Alison Schofield
@ 2026-08-08 5:52 ` Li Ming
0 siblings, 0 replies; 10+ messages in thread
From: Li Ming @ 2026-08-08 5:52 UTC (permalink / raw)
To: Alison Schofield
Cc: linux-cxl, Srirangan Madhavan, Davidlohr Bueso, Jonathan Cameron,
Dave Jiang, Vishal Verma, Ira Weiny
On 8/8/2026 8:05 AM, Alison Schofield wrote:
> Switch decoder target lookups walk the target array using interleave
> ways as the bound. The array is allocated with nr_targets entries, so a
> decoder whose interleave ways exceeds nr_targets reads and writes past
> the end of it. That happens during dport activation, during decoder
> target population, and when userspace reads the target_list attribute.
>
> Bounding by nr_targets alone is not correct either. Commit d6488fee6647
> ("cxl/port: Fix decoder initialization when nr_targets > interleave_ways")
> moved target population to interleave ways precisely so that targets a
> decoder does not use are left unpopulated. Neither field alone is a safe
> bound, so add a helper that returns the minimum of the two and use it at
> every walk of the target array.
>
> Valid configurations keep interleave ways within nr_targets, so this is
> not a behavior change for them.
>
> Fixes: 40ba17afdfab ("cxl/acpi: Introduce cxl_decoder objects")
> Fixes: d6488fee6647 ("cxl/port: Fix decoder initialization when nr_targets > interleave_ways")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-08 5:53 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 0:05 [PATCH v5 0/4] Harden HDM decoder enumeration Alison Schofield
2026-08-08 0:05 ` [PATCH v5 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets Alison Schofield
2026-08-08 0:17 ` sashiko-bot
2026-08-08 0:36 ` Alison Schofield
2026-08-08 5:50 ` Li Ming
2026-08-08 0:05 ` [PATCH v5 2/4] cxl/hdm: Make switch decoder target parsing endian-safe Alison Schofield
2026-08-08 5:51 ` Li Ming
2026-08-08 0:05 ` [PATCH v5 3/4] cxl/hdm: Restore commit_end when decoder enumeration fails Alison Schofield
2026-08-08 0:05 ` [PATCH v5 4/4] cxl/port: Bound switch decoder target array access by nr_targets Alison Schofield
2026-08-08 5:52 ` Li Ming
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.