[PATCH v5 0/7] cxl: Sashiko bug fixes

[PATCH v5 0/7] cxl: Sashiko bug fixes

[Richard Cheng]

Seven independent, pre-existing bugs in the CXL core, reported by
sashiko.

Patch 1: Get/Set Feature derive each mailbox command's offset from the
starting offset plus the amount of data already transferred, then store
it in a 16-bit field. A large offset/count supplied through fwctl can
cause a later offset to exceed the representable feature extent and be
truncated by cpu_to_le16(), targeting the wrong feature data. Reject
invalid ranges up front.

Change cxl_get_feature() to return ssize_t so invalid input and mailbox
failures are reported as negative errno instead of being conflated with
a zero-byte result. Update all EDAC callers for the signed return
contract while preserving the existing fwctl RPC response behavior.

Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
the first fully-mapped partition, silently skipping unmapped poison in
all later partitions. Skip that partition instead.

Patch 3: the same function tolerated the -EFAULT a RAM partition returns
for Get Poison List but left it in rc, so a benign fault on the last
scanned partition surfaced as a spurious read failure. Clear rc, as
poison_by_decoder() already does.

Patch 4: the same function also ignored the ctx->offset handoff from
poison_by_decoder() and derived its scan start from the highest DPA
allocation, so the DPA of allocated-but-uncommitted decoders was never
scanned by either phase. Resume the sweep at ctx->offset.

Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
query, so an earlier partition's failure was masked by a later success
and unscanned poison was reported as a clean list. Stop on any error
not tolerated as a RAM -EFAULT.

Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
was assigned to a partition. Malformed device DPA metadata can leave
dpa_res set while part remains -1, causing a poison scan to access
before the partition array. Reject such decoders before the lookup.


Changes since v4 [1]:
- Patch 7: New patch

[1]:
https://lore.kernel.org/linux-cxl/20260812112107.56181-1-icheng@nvidia.com/

Richard Cheng (7):
  cxl/features: Reject feature offset that overflows 16-bit field
  cxl/region: Scan all partitions for unmapped poison
  cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
  cxl/region: Start unmapped poison scan at the committed decoder
    boundary
  cxl/memdev: Don't overwrite the error from an earlier partition poison
    query
  cxl/region: Reject poison scan for decoder without a partition
  cxl/fwctl: Propagate feature RPC delivery errors

 drivers/cxl/core/core.h     |  8 ++++----
 drivers/cxl/core/edac.c     | 20 +++++++++++++++-----
 drivers/cxl/core/features.c | 37 +++++++++++++++++++++++++------------
 drivers/cxl/core/memdev.c   |  2 ++
 drivers/cxl/core/region.c   | 15 ++++++++-------
 5 files changed, 54 insertions(+), 28 deletions(-)


base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.0

[PATCH v5 1/7] cxl/features: Reject feature offset that overflows 16-bit field

[Richard Cheng]

cxl_get_feature() and cxl_set_feature() build each mailbox command's
offset from the starting offset plus the amount of data already
transferred, then store it in a 16-bit field. A user-controlled fwctl
offset and transfer size can exceed the feature extent, allowing a later
offset to be truncated by cpu_to_le16() and target the wrong feature
data.

Reject requests whose transfer size exceeds the remaining 16-bit feature
range. Express the check as "size > U16_MAX - offset" so the validation
itself cannot wrap on 32-bit systems.

Change cxl_get_feature() to return ssize_t so invalid input and mailbox
failures are reported as negative errno rather than being conflated with
a zero-byte result. Update the EDAC callers to handle negative results.
Keep fwctl behavior unchanged by translating helper failures to the same
header-only RPC response carrying the CXL mailbox return code.

Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
Fixes: 14d502cc2718 ("cxl/mbox: Add SET_FEATURE mailbox command")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/core.h     |  8 ++++----
 drivers/cxl/core/edac.c     | 20 +++++++++++++++-----
 drivers/cxl/core/features.c | 28 ++++++++++++++++++----------
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 35eaf636adc9..bb380ec6daeb 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -217,10 +217,10 @@ int cxl_port_get_possible_dports(struct cxl_port *port);
 #ifdef CONFIG_CXL_FEATURES
 struct cxl_feat_entry *
 cxl_feature_info(struct cxl_features_state *cxlfs, const uuid_t *uuid);
-size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
-		       enum cxl_get_feat_selection selection,
-		       void *feat_out, size_t feat_out_size, u16 offset,
-		       u16 *return_code);
+ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
+			enum cxl_get_feat_selection selection,
+			void *feat_out, size_t feat_out_size, u16 offset,
+			u16 *return_code);
 int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		    u8 feat_version, const void *feat_data,
 		    size_t feat_data_size, u32 feat_flag, u16 offset,
diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index b321971fef58..f1df4b5cfe5b 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -78,7 +78,7 @@ static int cxl_mem_scrub_get_attrbs(struct cxl_mailbox *cxl_mbox, u8 *cap,
 				    u16 *cycle, u8 *flags, u8 *min_cycle)
 {
 	size_t rd_data_size = sizeof(struct cxl_scrub_rd_attrbs);
-	size_t data_size;
+	ssize_t data_size;
 	struct cxl_scrub_rd_attrbs *rd_attrbs __free(kfree) =
 		kzalloc(rd_data_size, GFP_KERNEL);
 	if (!rd_attrbs)
@@ -87,6 +87,8 @@ static int cxl_mem_scrub_get_attrbs(struct cxl_mailbox *cxl_mbox, u8 *cap,
 	data_size = cxl_get_feature(cxl_mbox, &CXL_FEAT_PATROL_SCRUB_UUID,
 				    CXL_GET_FEAT_SEL_CURRENT_VALUE, rd_attrbs,
 				    rd_data_size, 0, NULL);
+	if (data_size < 0)
+		return data_size;
 	if (!data_size)
 		return -EIO;
 
@@ -551,7 +553,7 @@ static int cxl_mem_ecs_get_attrbs(struct device *dev,
 	struct cxl_mailbox *cxl_mbox = &cxlmd->cxlds->cxl_mbox;
 	struct cxl_ecs_fru_rd_attrbs *fru_rd_attrbs;
 	size_t rd_data_size;
-	size_t data_size;
+	ssize_t data_size;
 
 	rd_data_size = cxl_ecs_ctx->get_feat_size;
 
@@ -563,6 +565,8 @@ static int cxl_mem_ecs_get_attrbs(struct device *dev,
 	data_size = cxl_get_feature(cxl_mbox, &CXL_FEAT_ECS_UUID,
 				    CXL_GET_FEAT_SEL_CURRENT_VALUE, rd_attrbs,
 				    rd_data_size, 0, NULL);
+	if (data_size < 0)
+		return data_size;
 	if (!data_size)
 		return -EIO;
 
@@ -583,7 +587,7 @@ static int cxl_mem_ecs_set_attrbs(struct device *dev,
 	struct cxl_ecs_fru_wr_attrbs *fru_wr_attrbs;
 	size_t rd_data_size, wr_data_size;
 	u16 num_media_frus, count;
-	size_t data_size;
+	ssize_t data_size;
 
 	num_media_frus = cxl_ecs_ctx->num_media_frus;
 	rd_data_size = cxl_ecs_ctx->get_feat_size;
@@ -596,6 +600,8 @@ static int cxl_mem_ecs_set_attrbs(struct device *dev,
 	data_size = cxl_get_feature(cxl_mbox, &CXL_FEAT_ECS_UUID,
 				    CXL_GET_FEAT_SEL_CURRENT_VALUE, rd_attrbs,
 				    rd_data_size, 0, NULL);
+	if (data_size < 0)
+		return data_size;
 	if (!data_size)
 		return -EIO;
 
@@ -1264,7 +1270,7 @@ cxl_mem_sparing_get_attrbs(struct cxl_mem_sparing_context *cxl_sparing_ctx)
 	struct cxl_memdev *cxlmd = cxl_sparing_ctx->cxlmd;
 	struct cxl_mailbox *cxl_mbox = &cxlmd->cxlds->cxl_mbox;
 	u16 restriction_flags;
-	size_t data_size;
+	ssize_t data_size;
 	u16 return_code;
 	struct cxl_memdev_sparing_rd_attrbs *rd_attrbs __free(kfree) =
 		kzalloc(rd_data_size, GFP_KERNEL);
@@ -1274,6 +1280,8 @@ cxl_mem_sparing_get_attrbs(struct cxl_mem_sparing_context *cxl_sparing_ctx)
 	data_size = cxl_get_feature(cxl_mbox, &cxl_sparing_ctx->repair_uuid,
 				    CXL_GET_FEAT_SEL_CURRENT_VALUE, rd_attrbs,
 				    rd_data_size, 0, &return_code);
+	if (data_size < 0)
+		return data_size;
 	if (!data_size)
 		return -EIO;
 
@@ -1750,7 +1758,7 @@ static int cxl_mem_ppr_get_attrbs(struct cxl_ppr_context *cxl_ppr_ctx)
 	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
 	struct cxl_mailbox *cxl_mbox = &cxlmd->cxlds->cxl_mbox;
 	u16 restriction_flags;
-	size_t data_size;
+	ssize_t data_size;
 	u16 return_code;
 
 	struct cxl_memdev_ppr_rd_attrbs *rd_attrbs __free(kfree) =
@@ -1761,6 +1769,8 @@ static int cxl_mem_ppr_get_attrbs(struct cxl_ppr_context *cxl_ppr_ctx)
 	data_size = cxl_get_feature(cxl_mbox, &cxl_ppr_ctx->repair_uuid,
 				    CXL_GET_FEAT_SEL_CURRENT_VALUE, rd_attrbs,
 				    rd_data_size, 0, &return_code);
+	if (data_size < 0)
+		return data_size;
 	if (!data_size)
 		return -EIO;
 
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index ba6d2a5acb74..8d44ce829497 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -220,10 +220,10 @@ int devm_cxl_setup_features(struct cxl_dev_state *cxlds)
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_setup_features, "CXL");
 
-size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
-		       enum cxl_get_feat_selection selection,
-		       void *feat_out, size_t feat_out_size, u16 offset,
-		       u16 *return_code)
+ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
+			enum cxl_get_feat_selection selection,
+			void *feat_out, size_t feat_out_size, u16 offset,
+			u16 *return_code)
 {
 	size_t data_to_rd_size;
 	struct cxl_mbox_get_feat_in pi;
@@ -235,7 +235,10 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		*return_code = CXL_MBOX_CMD_RC_INPUT;
 
 	if (!feat_out || !feat_out_size)
-		return 0;
+		return -EINVAL;
+
+	if (feat_out_size > U16_MAX - offset)
+		return -EINVAL;
 
 	uuid_copy(&pi.uuid, feat_uuid);
 	pi.selection = selection;
@@ -259,7 +262,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		if (rc < 0 || !mbox_cmd.size_out) {
 			if (return_code)
 				*return_code = mbox_cmd.return_code;
-			return 0;
+			return rc < 0 ? rc : -EIO;
 		}
 		data_rcvd_size += mbox_cmd.size_out;
 	} while (data_rcvd_size < feat_out_size);
@@ -288,6 +291,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
 	if (return_code)
 		*return_code = CXL_MBOX_CMD_RC_INPUT;
 
+	if (feat_data_size > U16_MAX - offset)
+		return -EINVAL;
+
 	struct cxl_mbox_set_feat_in *pi __free(kfree) =
 			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
 	if (!pi)
@@ -462,6 +468,7 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
 	const struct cxl_mbox_get_feat_in *feat_in;
 	u16 offset, count, return_code;
 	size_t out_size = *out_len;
+	ssize_t data_size;
 
 	if (rpc_in->op_size != sizeof(*feat_in))
 		return ERR_PTR(-EINVAL);
@@ -482,16 +489,17 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
 	if (!rpc_out)
 		return ERR_PTR(-ENOMEM);
 
-	out_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
-				   feat_in->selection, rpc_out->payload,
-				   count, offset, &return_code);
+	data_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
+				    feat_in->selection, rpc_out->payload,
+				    count, offset, &return_code);
 	*out_len = sizeof(struct fwctl_rpc_cxl_out);
-	if (!out_size) {
+	if (data_size <= 0) {
 		rpc_out->size = 0;
 		rpc_out->retval = return_code;
 		return no_free_ptr(rpc_out);
 	}
 
+	out_size = data_size;
 	rpc_out->size = out_size;
 	rpc_out->retval = CXL_MBOX_CMD_RC_SUCCESS;
 	*out_len += out_size;
-- 
2.43.0

[PATCH v5 2/7] cxl/region: Scan all partitions for unmapped poison

[Richard Cheng]

cxl_get_poison_unmapped() sweeps the unmapped tail of each partition
from ctx->part onward. A fully-mapped partition has no unmapped tail,
it's a normal per-partition state, but the loop treated it with break,
aborting the whole sweep and silently skipping unmapped poison in all
later partition. Use continue so a fully-mapped partition is skipped and
later partitions are still scanned.

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Tested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v1->v2:
    - Tweak commit message
---
 drivers/cxl/core/region.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 27e63e6dab7c..b03f30c04d35 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2928,7 +2928,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 			offset = res->start;
 		length = res->end - offset + 1;
 		if (!length)
-			break;
+			continue;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
 		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
 			continue;
-- 
2.43.0

[PATCH v5 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

[Richard Cheng]

cxl_get_poison_unmapped() tolerates the -EFAULT a RAM partition returns
for Get Poison List by skipping that partition, but left rc holding the
error. If the tolerated RAM fault was the last poison query before the
loop ended, the function returned a spurious -EFAULT and the poison-list
read failed even though enumeration succeeded. Reset rc to 0 when
tolerating the fault, matching poison_by_decoder().

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b03f30c04d35..ddf12075b95c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2930,8 +2930,10 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 		if (!length)
 			continue;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
-		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
+		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM) {
+			rc = 0;
 			continue;
+		}
 		if (rc)
 			break;
 	}
-- 
2.43.0

[PATCH v5 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary

[Richard Cheng]

poison_by_decoder() stops at the last committed decoder and records the
handoff in ctx->offset, but cxl_get_poison_unmapped() ignores it and
starts after the highest DPA allocation instead. Allocation exist for
uncommitted decoders too, so their DPA is skipped by both phases and
poison there is never reported. Resume the scan at ctx->offset, and scan
later partitions in full, restoring the pre-rewrite behavior.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v1->v2:
    - New added patch ( sashiko's report )
---
 drivers/cxl/core/region.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index ddf12075b95c..b7dc5d4988da 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2907,7 +2907,6 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	const struct resource *res;
-	struct resource *p, *last;
 	u64 offset, length;
 	int rc = 0;
 
@@ -2920,10 +2919,8 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 	 */
 	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
 		res = &cxlds->part[i].res;
-		for (p = res->child, last = NULL; p; p = p->sibling)
-			last = p;
-		if (last)
-			offset = last->end + 1;
+		if (i == ctx->part)
+			offset = ctx->offset;
 		else
 			offset = res->start;
 		length = res->end - offset + 1;
-- 
2.43.0

[PATCH v5 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query

[Richard Cheng]

cxl_get_poison_by_memdev() queries Get Poison List per partition but
never checks the result inside the loop, so a later partition's success
overwrites an earlier partition's failure and the whole scan reports
success while that partition's poison went unlisted. Before the loop
conversion the PMEM query returned early on error. Stop the loop on any
error not already tolerated as a RAM -EFAULT.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v1->v2:
    - New added patch ( sashiko-bot's report )
---
 drivers/cxl/core/memdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index b3419df586b9..e39b3d13fd56 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -231,6 +231,8 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
 		 */
 		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
 			rc = 0;
+		if (rc)
+			break;
 	}
 	return rc;
 }
-- 
2.43.0

[PATCH v5 6/7] cxl/region: Reject poison scan for decoder without a partition

[Richard Cheng]

__cxl_dpa_reserve() may leave cxled->part at -1 when a decoder's DPA
range doesn't map to any reported partition, while still keeping
dpa_res. poison_by_decoder() then indexes cxlds->part[-1], causing an
out-of-bounds read when poison collection is triggered.

Return -ENODEV before accessing the partition array when no partition was
assigned.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b7dc5d4988da..afe3fb57b7fe 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2954,6 +2954,8 @@ static int poison_by_decoder(struct device *dev, void *arg)
 	cxled = to_cxl_endpoint_decoder(dev);
 	if (!cxled->dpa_res)
 		return rc;
+	if (cxled->part < 0)
+		return -ENODEV;
 
 	cxlmd = cxled_to_memdev(cxled);
 	cxlds = cxlmd->cxlds;
-- 
2.43.0

[PATCH v5 7/7] cxl/fwctl: Propagate feature RPC delivery errors

[Richard Cheng]

FWCTL_RPC requires delivery failures to be returned as ioctl errors,
while device errors are reported in the output. Get and Set Feature
instead converted all failures into normal responses, sometimes with a
SUCCESS device status.

Initialize the return code to SUCCESS. When the helper fails without a
device error code, return its errno. Continue reporting actual device
errors through rpc_out->retval.

Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/features.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 8d44ce829497..30e00faaf3a9 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -232,7 +232,7 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 	int rc;
 
 	if (return_code)
-		*return_code = CXL_MBOX_CMD_RC_INPUT;
+		*return_code = CXL_MBOX_CMD_RC_SUCCESS;
 
 	if (!feat_out || !feat_out_size)
 		return -EINVAL;
@@ -289,7 +289,7 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
 	size_t hdr_size;
 
 	if (return_code)
-		*return_code = CXL_MBOX_CMD_RC_INPUT;
+		*return_code = CXL_MBOX_CMD_RC_SUCCESS;
 
 	if (feat_data_size > U16_MAX - offset)
 		return -EINVAL;
@@ -492,6 +492,9 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
 	data_size = cxl_get_feature(cxl_mbox, &feat_in->uuid,
 				    feat_in->selection, rpc_out->payload,
 				    count, offset, &return_code);
+	if (data_size < 0 &&
+	    return_code == CXL_MBOX_CMD_RC_SUCCESS)
+		return ERR_PTR(data_size);
 	*out_len = sizeof(struct fwctl_rpc_cxl_out);
 	if (data_size <= 0) {
 		rpc_out->size = 0;
@@ -544,6 +547,8 @@ static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
 	rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
 			     feat_in->version, feat_in->feat_data,
 			     data_size, flags, offset, &return_code);
+	if (rc && return_code == CXL_MBOX_CMD_RC_SUCCESS)
+		return ERR_PTR(rc);
 	*out_len = sizeof(*rpc_out);
 	if (rc) {
 		rpc_out->retval = return_code;
-- 
2.43.0

Re: [PATCH v5 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds read and information leak via hardware-supplied poison record count.
--

--- Patch [4]: [PATCH v5 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary ---
commit 7ebf989f20ca7ca7009f85db447392e5d1e320f2
Author: Richard Cheng <icheng@nvidia.com>
Subject: cxl/region: Start unmapped poison scan at the committed decoder boundary

This commit fixes poison reporting for unmapped DPA ranges by resuming
the scan at the last committed decoder offset instead of skipping it.
It ensures poison in uncommitted decoder allocations is properly scanned
and reported.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2920,10 +2919,8 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  	 */
>  	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
>  		res = &cxlds->part[i].res;
> -		for (p = res->child, last = NULL; p; p = p->sibling)
> -			last = p;
> -		if (last)
> -			offset = last->end + 1;
> +		if (i == ctx->part)
> +			offset = ctx->offset;
>  		else
>  			offset = res->start;
>  		length = res->end - offset + 1;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the subsequent
call to cxl_mem_get_poison(), I noticed a potential out-of-bounds read in
drivers/cxl/core/mbox.c.

Does cxl_mem_get_poison() properly bounds check the hardware-supplied
poison record count?

In drivers/cxl/core/mbox.c:cxl_mem_get_poison(), the loop blindly uses
po->count to iterate:

		for (int i = 0; i < le16_to_cpu(po->count); i++)
			trace_cxl_poison(cxlmd, cxlr, &po->record[i],
					 po->flags, po->overflow_ts,
					 CXL_POISON_TRACE_LIST);

If a malicious or buggy CXL device returns a small payload
(e.g., meeting the 32-byte header min_out) but a large po->count
(e.g., 65535), the internal send command will succeed. 

Will this cause the loop to read past the end of the kvmalloc-backed
buffer and leak kernel memory into the ftrace ring buffer, or
potentially panic if it reaches an unmapped page?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813034538.13189-1-icheng@nvidia.com?part=4

Re: [PATCH v5 6/7] cxl/region: Reject poison scan for decoder without a partition

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Returning -ENODEV from poison_by_decoder() for unmapped decoders aborts the entire endpoint's poison list collection.
--

--- Patch [6]: [PATCH v5 6/7] cxl/region: Reject poison scan for decoder without a partition ---
commit 8d92957ad7710c4351ca4406f4bb3f3474e28442
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Reject poison scan for decoder without a partition

This commit aims to prevent an out-of-bounds array read in
poison_by_decoder() by returning early when a decoder has no assigned
partition. It checks if the partition is negative and returns -ENODEV
before indexing the partition array.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b7dc5d4988da7..afe3fb57b7fe2 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2954,6 +2954,8 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  	cxled = to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
>  		return rc;
> +	if (cxled->part < 0)
> +		return -ENODEV;

[Severity: Medium]
Will returning -ENODEV here abort the iteration for the entire endpoint?

Since poison_by_decoder() is used as a callback, returning a non-zero error
code causes device_for_each_child() to stop iterating immediately. This would
completely prevent the retrieval of poison data for all other valid, mapped
decoders on the same device.

Would it be better to return 0 instead, which would safely skip the unmapped
decoder but allow the poison scan to continue for the remaining decoders?

>  
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813034538.13189-1-icheng@nvidia.com?part=6

Re: [PATCH v5 1/7] cxl/features: Reject feature offset that overflows 16-bit field

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The bounds check designed to prevent 16-bit offset wrap-around is off by one, making the final byte of the feature space inaccessible.
--

--- Patch [1]: [PATCH v5 1/7] cxl/features: Reject feature offset that overflows 16-bit field ---
commit 48e82cdf4087a2ffbeb7023fda65d3ff4f9b9c6f
Author: Richard Cheng <icheng@nvidia.com>

cxl/features: Reject feature offset that overflows 16-bit field

This commit adds bounds checking to prevent 16-bit offset wrap-around in CXL
feature commands. It also changes cxl_get_feature() to return ssize_t to
properly report errors instead of conflating them with a zero-byte result.

> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index ba6d2a5acb74a..8d44ce8294978 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
[ ... ]
> @@ -235,7 +235,10 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
>  	if (!feat_out || !feat_out_size)
> -		return 0;
> +		return -EINVAL;
> +
> +	if (feat_out_size > U16_MAX - offset)
> +		return -EINVAL;

[Severity: Medium]
Does this check contain an off-by-one error that rejects valid accesses to the
last byte of the feature space?

If a request targets the final byte of the 64KB feature space with
offset = 65535 and feat_out_size = 1, this evaluates to 1 > 65535 - 65535
(1 > 0), which is true, incorrectly rejecting the single-byte transfer.

>  
>  	uuid_copy(&pi.uuid, feat_uuid);
>  	pi.selection = selection;
[ ... ]
> @@ -288,6 +291,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  	if (return_code)
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
> +	if (feat_data_size > U16_MAX - offset)
> +		return -EINVAL;
> +

[Severity: Medium]
Does this bounds check in cxl_set_feature() have the same off-by-one issue
restricting access to the last byte of the feature space?

>  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
>  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
>  	if (!pi)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813034538.13189-1-icheng@nvidia.com?part=1

Re: [PATCH v5 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `cxl_get_poison_by_memdev()` function silently masks hardware errors from early partitions due to a missing error check.
--

--- Patch [3]: [PATCH v5 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan ---
commit 4c4b3832747d23fff6bcb4486dc7105f329afd3d
Author: Richard Cheng <icheng@nvidia.com>
Subject: cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

This commit fixes an issue where a tolerated error from a RAM partition
causes the unmapped poison scan to fail. It correctly resets the return
code to 0 when ignoring the fault, matching the behavior of
poison_by_decoder().

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b03f30c04d350..ddf12075b95c0 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2930,8 +2930,10 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  		if (!length)
>  			continue;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> -		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
> +		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM) {
> +			rc = 0;
>  			continue;
> +		}
>  		if (rc)
>  			break;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this error
handling, I noticed a potential issue with similar logic in
cxl_get_poison_by_memdev() in drivers/cxl/core/memdev.c.

Does the loop in cxl_get_poison_by_memdev() mask hardware errors from
early partitions due to a missing error check?

drivers/cxl/core/memdev.c:cxl_get_poison_by_memdev() {
        ...
        for (int i = 0; i < cxlds->nr_partitions; i++) {
                ...
                rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
                /*
                 * Invalid Physical Address is not an error for
                 * volatile addresses. Device support is optional.
                 */
                if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
                        rc = 0;
        }
        return rc;
}

If cxl_mem_get_poison() returns an error on an early partition, the loop
continues without breaking. Could a success on a subsequent partition then
overwrite rc with 0, masking the retrieval failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813034538.13189-1-icheng@nvidia.com?part=3

Re: [PATCH v5 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to `cxlmd->endpoint` in `cxl_trigger_poison_list()` leads to a TOCTOU Use-After-Free if the device is concurrently unbound.
- [High] Missing bounds check on `po->count` in `cxl_mem_get_poison()` allows an out-of-bounds read of kernel memory.
- [Medium] `cxl_mem_get_poison()` fails to update the payload offset when retrieving subsequent poison records, resulting in duplicated traces and dropped records.
--

--- Patch [5]: [PATCH v5 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query ---
commit 68fad9eb0dea3e233e2489fa05fad1099307f601
Author: Richard Cheng <icheng@nvidia.com>

cxl/memdev: Don't overwrite the error from an earlier partition poison query

cxl_get_poison_by_memdev() currently doesn't check the loop result, allowing
successful partition scans to overwrite earlier failures. This patch breaks
the loop on any unhandled error to properly report failures.

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index b3419df586b9c..e39b3d13fd566 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -231,6 +231,8 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
>  		 */
>  		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
>  			rc = 0;
> +		if (rc)
> +			break;
>  	}
>  	return rc;
>  }

While reviewing this, I noticed a few pre-existing issues in related code:

drivers/cxl/core/memdev.c:cxl_trigger_poison_list() {
    ...
    int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
    {
        struct cxl_port *port;
        int rc;

        port = cxlmd->endpoint;
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but when a root user writes to the
trigger_poison_list sysfs attribute of a cxl_memdev device, it calls
cxl_trigger_poison_list(), which reads cxlmd->endpoint locklessly.

Concurrently, the parent CXL port can be unbound.

The unbind triggers the devm callback delete_endpoint(), which clears
cxlmd->endpoint to NULL and releases the port's reference, freeing it.

Since the sysfs thread has already loaded the pointer, it proceeds to use the
freed port pointer in calls to cxl_num_decoders_committed(port) and
cxl_get_poison_by_endpoint(port).

Does lockless access to cxlmd->endpoint in cxl_trigger_poison_list() lead to a
TOCTOU Use-After-Free if the device is concurrently unbound?


drivers/cxl/core/mbox.c:cxl_mem_get_poison() {
    ...
        if (!le16_to_cpu(po->count)) {
            dev_dbg(&cxlmd->dev, "Poison empty payload!\n");
            break;
        }

        for (int i = 0; i < le16_to_cpu(po->count); i++)
            trace_cxl_poison(cxlmd, cxlr, &po->record[i],
                     po->flags, po->overflow_ts,
                     CXL_POISON_TRACE_LIST);
    ...
}

[Severity: High]
This is a pre-existing issue, but in cxl_mem_get_poison(), the driver sends a
GET_POISON_LIST command.

While cxl_internal_send_cmd() validates that the returned payload is at least
the size of the response header, it does not validate that it is large enough
to contain le16_to_cpu(po->count) records.

A malicious or buggy CXL device could return a small payload (e.g., 24 bytes)
but a large po->count (e.g., 65535).

The driver iterates po->count times, reading po->record[i] from the
mds->poison.list_out buffer (which is allocated as cxl_mbox->payload_size).
This will read past the end of the allocation.

Is there a missing bounds check on po->count in cxl_mem_get_poison() that
allows an out-of-bounds read of kernel memory?


drivers/cxl/core/mbox.c:cxl_mem_get_poison() {
    ...
    po = mds->poison.list_out;
    pi.offset = cpu_to_le64(offset);
    pi.length = cpu_to_le64(len / CXL_POISON_LEN_MULT);

    do {
        struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd){
            .opcode = CXL_MBOX_OP_GET_POISON,
            .size_in = sizeof(pi),
            .payload_in = &pi,
            .size_out = cxl_mbox->payload_size,
            .payload_out = po,
            .min_out = struct_size(po, record, 0),
        };

        rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
    ...
}

[Severity: Medium]
This isn't a bug introduced by this patch, but according to the CXL 3.0
specification, if the device sets the 'More Poison Records' flag, the host
must issue subsequent Get Poison List commands with an updated Offset and
Length.

In cxl_mem_get_poison(), pi.offset and pi.length are populated before the
do-while loop.

Because they are never updated inside the loop, the driver sends the identical
payload in every iteration.

A stateless CXL device will respond with the identical first chunk of poison
records, continuously setting the MORE flag.

The driver will log these duplicate records until it artificially hits
mds->poison.max_errors, dropping the remaining records.

Does cxl_mem_get_poison() fail to update the payload offset when retrieving
subsequent poison records, resulting in duplicated traces and dropped records?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813034538.13189-1-icheng@nvidia.com?part=5