* [PATCH v7 0/7] cxl: Sashiko bug fixes
@ 2026-09-02 5:38 Richard Cheng
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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.
Patch 7: the Get and Set Feature fwctl handlers converted all helper
failures into normal RPC responses, sometimes with a SUCCESS device
status. Propagate delivery failures as ioctl errors while continuing to
report actual device errors through rpc_out->retval.
A nonzero short Get Feature response is valid when Offset + Count runs
past the end of the feature. Preserve the returned bytes as a successful
partial transfer, reject unexpected zero-length success responses, and
require fixed-format EDAC callers to receive their complete attribute
structures before consuming them.
Changes since v6 [1]:
- Patch 7: Remove redundant braces. (Dave Jiang)
[1]:
https://lore.kernel.org/linux-cxl/20260826014508.9989-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 | 30 +++++++++++++-------
drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
drivers/cxl/core/memdev.c | 2 ++
drivers/cxl/core/region.c | 15 +++++-----
5 files changed, 71 insertions(+), 40 deletions(-)
base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
--
2.53.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-04 5:23 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 2/7] cxl/region: Scan all partitions for unmapped poison Richard Cheng
` (7 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 2/7] cxl/region: Scan all partitions for unmapped poison
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-02 5:38 ` [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
` (6 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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>
---
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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
2026-09-02 5:38 ` [PATCH v7 2/7] cxl/region: Scan all partitions for unmapped poison Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-04 5:25 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
` (5 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (2 preceding siblings ...)
2026-09-02 5:38 ` [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-02 5:38 ` [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (3 preceding siblings ...)
2026-09-02 5:38 ` [PATCH v7 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-04 5:31 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=5
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (4 preceding siblings ...)
2026-09-02 5:38 ` [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-02 5:52 ` sashiko-bot
2026-09-04 5:57 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors Richard Cheng
` (2 subsequent siblings)
8 siblings, 2 replies; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (5 preceding siblings ...)
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
@ 2026-09-02 5:38 ` Richard Cheng
2026-09-04 5:21 ` Alison Schofield
2026-09-04 5:20 ` [PATCH v7 0/7] cxl: Sashiko bug fixes Alison Schofield
2026-09-04 16:19 ` Dave Jiang
8 siblings, 1 reply; 16+ messages in thread
From: Richard Cheng @ 2026-09-02 5:38 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak, 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.
CXL permits Get Feature to return a nonzero short payload when Offset +
Count runs past the end of the Feature. cxl_internal_send_cmd() reports
that response as -EIO, so preserve the returned bytes as a successful
partial transfer. Fixed-format EDAC callers still require complete
attribute structures, so reject partial payloads before consuming them.
Map an unexpected zero-length result with a SUCCESS device status to
-EIO.
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/edac.c | 10 +++++-----
drivers/cxl/core/features.c | 28 +++++++++++++++++++---------
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index f1df4b5cfe5b..34b81e8dfbbb 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -89,7 +89,7 @@ static int cxl_mem_scrub_get_attrbs(struct cxl_mailbox *cxl_mbox, u8 *cap,
rd_data_size, 0, NULL);
if (data_size < 0)
return data_size;
- if (!data_size)
+ if ((size_t)data_size != rd_data_size)
return -EIO;
*cap = rd_attrbs->scrub_cycle_cap;
@@ -567,7 +567,7 @@ static int cxl_mem_ecs_get_attrbs(struct device *dev,
rd_data_size, 0, NULL);
if (data_size < 0)
return data_size;
- if (!data_size)
+ if ((size_t)data_size != rd_data_size)
return -EIO;
fru_rd_attrbs = rd_attrbs->fru_attrbs;
@@ -602,7 +602,7 @@ static int cxl_mem_ecs_set_attrbs(struct device *dev,
rd_data_size, 0, NULL);
if (data_size < 0)
return data_size;
- if (!data_size)
+ if ((size_t)data_size != rd_data_size)
return -EIO;
struct cxl_ecs_wr_attrbs *wr_attrbs __free(kvfree) =
@@ -1282,7 +1282,7 @@ cxl_mem_sparing_get_attrbs(struct cxl_mem_sparing_context *cxl_sparing_ctx)
rd_data_size, 0, &return_code);
if (data_size < 0)
return data_size;
- if (!data_size)
+ if ((size_t)data_size != rd_data_size)
return -EIO;
cxl_sparing_ctx->op_class = rd_attrbs->hdr.op_class;
@@ -1771,7 +1771,7 @@ static int cxl_mem_ppr_get_attrbs(struct cxl_ppr_context *cxl_ppr_ctx)
rd_data_size, 0, &return_code);
if (data_size < 0)
return data_size;
- if (!data_size)
+ if ((size_t)data_size != rd_data_size)
return -EIO;
cxl_ppr_ctx->op_class = rd_attrbs->hdr.op_class;
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 8d44ce829497..95f47193fb61 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;
@@ -259,6 +259,17 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
.min_out = data_to_rd_size,
};
rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+ /*
+ * Per CXL r4.0 8.2.10.6.2, when Offset + Count runs past the
+ * end of the Feature the device returns only the bytes up to
+ * the Feature size. cxl_internal_send_cmd() reports that as
+ * -EIO with a short payload, so stop and return what arrived.
+ */
+ if (rc == -EIO && mbox_cmd.size_out &&
+ mbox_cmd.size_out < data_to_rd_size) {
+ data_rcvd_size += mbox_cmd.size_out;
+ break;
+ }
if (rc < 0 || !mbox_cmd.size_out) {
if (return_code)
*return_code = mbox_cmd.return_code;
@@ -267,9 +278,6 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
data_rcvd_size += mbox_cmd.size_out;
} while (data_rcvd_size < feat_out_size);
- if (return_code)
- *return_code = CXL_MBOX_CMD_RC_SUCCESS;
-
return data_rcvd_size;
}
@@ -289,7 +297,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;
@@ -340,11 +348,8 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
}
data_sent_size += data_in_size;
- if (data_sent_size >= feat_data_size) {
- if (return_code)
- *return_code = CXL_MBOX_CMD_RC_SUCCESS;
+ if (data_sent_size >= feat_data_size)
return 0;
- }
if ((feat_data_size - data_sent_size) <= (cxl_mbox->payload_size - hdr_size)) {
data_in_size = feat_data_size - data_sent_size;
@@ -492,6 +497,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 ?: -EIO);
*out_len = sizeof(struct fwctl_rpc_cxl_out);
if (data_size <= 0) {
rpc_out->size = 0;
@@ -544,6 +552,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.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
@ 2026-09-02 5:52 ` sashiko-bot
2026-09-04 5:57 ` Alison Schofield
1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-02 5:52 UTC (permalink / raw)
To: Richard Cheng; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Returning -ENODEV in poison_by_decoder() incorrectly aborts the iteration over the port's decoders, failing the entire poison scan.
Pre-existing issues:
- [High] The function `cxl_mem_get_poison` does not validate that the number of poison records returned by the device (`po->count`) fits within the allocated buffer, leading to an out-of-bounds read and information leak.
--
commit 38cab4750fe29ee2e45310ad8ff37111d581eb8d
Author: Richard Cheng <icheng@nvidia.com>
Subject: cxl/region: Reject poison scan for decoder without a partition
The __cxl_dpa_reserve() function may leave cxled->part at -1 when a
decoder's DPA range doesn't map to any reported partition. This commit
returns -ENODEV before accessing the partition array when no partition
was assigned.
> 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]
Does returning -ENODEV in poison_by_decoder() incorrectly abort the iteration
over the port's decoders?
When a CXL port has an endpoint decoder that is not mapped to any partition,
the device_for_each_child() iteration will abort prematurely on the first
unmapped decoder. This prevents the remaining valid decoders from being
scanned for poison and skips cxl_get_poison_unmapped() entirely.
Could this return 0 instead to skip the unmapped decoder while allowing the
overall poison scan to continue?
This isn't a bug introduced by this patch, but I noticed another problem in
the cxl_mem_get_poison() function called later in this path:
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]
Does this code overflow po->record[] if a buggy or malicious CXL device
returns a large po->count value?
The po->count value from the device payload is used to index the array
without being validated against the size of the list_out buffer allocated
via kvmalloc. If the firmware returns a large count value alongside a
small actual payload size, this could lead to an out-of-bounds read.
Can this leak adjacent kernel memory to the ftrace ring buffer or trigger a
kernel oops if the read crosses into an unmapped page?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902053839.25595-1-icheng@nvidia.com?part=6
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (6 preceding siblings ...)
2026-09-02 5:38 ` [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors Richard Cheng
@ 2026-09-04 5:20 ` Alison Schofield
2026-09-04 16:19 ` Dave Jiang
8 siblings, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:20 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:32PM +0800, Richard Cheng wrote:
> Seven independent, pre-existing bugs in the CXL core, reported by
> sashiko.
Hi Richard,
I finally got around to looking into all this poisonous code ;)
because I had an inkling that they couldn't be independent nor
coincidence based on that common Fixes Tag. That actually is
the issue, aka the finding.
be5cbd084027 is described as an enum removal. Mostly it did that,
CXL_DECODER_PMEM became CXL_PARTMODE_PMEM and nothing else moves.
But three functions were not renames, they were rewritten.
cxl_get_poison_by_memdev() two if blocks became one loop
cxl_get_poison_unmapped() rewritten, no line survives
poison_by_decoder() gained an unguarded part[] index
If you line up your patches against the pre-rewrite code you'll see
that each one is putting back a statement the rewrite dropped:
patch 2 'if (!length) return 0' was correct because pmem was the
last partition. It became a mid-loop break.
patch 3 the RAM branch cleared rc on a tolerated -EFAULT. The
rewrite continues without clearing it.
patch 4 the function consumed ctx->offset. The rewrite derives
the resume point from the resource child list instead --
note poison_by_decoder() still computes ctx->offset and
nothing reads it. That dead store is the tell.
patch 5 'if (rc) return rc' lived inside the pmem if block. When
the two blocks were merged into a loop it went with the
block and was never re-added.
patch 6 be5cbd084027 added six new cxlds->part[cxled->part]
dereferences and guarded five of them against part < 0.
In cxl_region_attach() it reordered the existing checks to
get the guard ahead of the index. poison_by_decoder() is
the one site it missed.
BTW - I did confirm it was not a merge issue.
Here's what I'm thinking. DaveJ can pluck patch 1 and 7 separately and
you can take another pass at this as a 'Poison Repair Set'.
I walked cxl_get_poison_unmapped() against the pre-be5cbd084027 form, but
not the other 2 rewritten functions. It would be good if you can do that.
so we can be sure nothing else is missing. Doing that, along with
addressing Sashiko citings, will get us to the finish line on this one.
I'm going to reply inline to Patches 5 and 6, but here is the
highlights:
- Patches 2, 3 and 4 together restore cxl_get_poison_unmapped()
and improve upon it. It is partition-generic now and no longer issues
a zero-length Get Poison List.
- Patch 5 fixes the masked error but loses the poison records that the
pre-rewrite code collected before it reported the failure.
- Patch 6 fixes the out-of-bounds read by failing the whole scan for
the memdev, where the pre-rewrite code scanned that decoder and
carried on.
-- Alison
>
> 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.
>
> Patch 7: the Get and Set Feature fwctl handlers converted all helper
> failures into normal RPC responses, sometimes with a SUCCESS device
> status. Propagate delivery failures as ioctl errors while continuing to
> report actual device errors through rpc_out->retval.
>
> A nonzero short Get Feature response is valid when Offset + Count runs
> past the end of the feature. Preserve the returned bytes as a successful
> partial transfer, reject unexpected zero-length success responses, and
> require fixed-format EDAC callers to receive their complete attribute
> structures before consuming them.
>
> Changes since v6 [1]:
> - Patch 7: Remove redundant braces. (Dave Jiang)
>
> [1]:
> https://lore.kernel.org/linux-cxl/20260826014508.9989-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 | 30 +++++++++++++-------
> drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
> drivers/cxl/core/memdev.c | 2 ++
> drivers/cxl/core/region.c | 15 +++++-----
> 5 files changed, 71 insertions(+), 40 deletions(-)
>
>
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors
2026-09-02 5:38 ` [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors Richard Cheng
@ 2026-09-04 5:21 ` Alison Schofield
0 siblings, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:21 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:39PM +0800, Richard Cheng wrote:
> 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.
>
> CXL permits Get Feature to return a nonzero short payload when Offset +
> Count runs past the end of the Feature. cxl_internal_send_cmd() reports
> that response as -EIO, so preserve the returned bytes as a successful
> partial transfer. Fixed-format EDAC callers still require complete
> attribute structures, so reject partial payloads before consuming them.
>
> Map an unexpected zero-length result with a SUCCESS device status to
> -EIO.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
> 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/edac.c | 10 +++++-----
> drivers/cxl/core/features.c | 28 +++++++++++++++++++---------
> 2 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index f1df4b5cfe5b..34b81e8dfbbb 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -89,7 +89,7 @@ static int cxl_mem_scrub_get_attrbs(struct cxl_mailbox *cxl_mbox, u8 *cap,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> *cap = rd_attrbs->scrub_cycle_cap;
> @@ -567,7 +567,7 @@ static int cxl_mem_ecs_get_attrbs(struct device *dev,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> fru_rd_attrbs = rd_attrbs->fru_attrbs;
> @@ -602,7 +602,7 @@ static int cxl_mem_ecs_set_attrbs(struct device *dev,
> rd_data_size, 0, NULL);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> struct cxl_ecs_wr_attrbs *wr_attrbs __free(kvfree) =
> @@ -1282,7 +1282,7 @@ cxl_mem_sparing_get_attrbs(struct cxl_mem_sparing_context *cxl_sparing_ctx)
> rd_data_size, 0, &return_code);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> cxl_sparing_ctx->op_class = rd_attrbs->hdr.op_class;
> @@ -1771,7 +1771,7 @@ static int cxl_mem_ppr_get_attrbs(struct cxl_ppr_context *cxl_ppr_ctx)
> rd_data_size, 0, &return_code);
> if (data_size < 0)
> return data_size;
> - if (!data_size)
> + if ((size_t)data_size != rd_data_size)
> return -EIO;
>
> cxl_ppr_ctx->op_class = rd_attrbs->hdr.op_class;
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 8d44ce829497..95f47193fb61 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;
> @@ -259,6 +259,17 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> .min_out = data_to_rd_size,
> };
> rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> + /*
> + * Per CXL r4.0 8.2.10.6.2, when Offset + Count runs past the
> + * end of the Feature the device returns only the bytes up to
> + * the Feature size. cxl_internal_send_cmd() reports that as
> + * -EIO with a short payload, so stop and return what arrived.
> + */
> + if (rc == -EIO && mbox_cmd.size_out &&
> + mbox_cmd.size_out < data_to_rd_size) {
> + data_rcvd_size += mbox_cmd.size_out;
> + break;
> + }
> if (rc < 0 || !mbox_cmd.size_out) {
> if (return_code)
> *return_code = mbox_cmd.return_code;
> @@ -267,9 +278,6 @@ ssize_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> data_rcvd_size += mbox_cmd.size_out;
> } while (data_rcvd_size < feat_out_size);
>
> - if (return_code)
> - *return_code = CXL_MBOX_CMD_RC_SUCCESS;
> -
> return data_rcvd_size;
> }
>
> @@ -289,7 +297,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;
> @@ -340,11 +348,8 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
> }
>
> data_sent_size += data_in_size;
> - if (data_sent_size >= feat_data_size) {
> - if (return_code)
> - *return_code = CXL_MBOX_CMD_RC_SUCCESS;
> + if (data_sent_size >= feat_data_size)
> return 0;
> - }
>
> if ((feat_data_size - data_sent_size) <= (cxl_mbox->payload_size - hdr_size)) {
> data_in_size = feat_data_size - data_sent_size;
> @@ -492,6 +497,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 ?: -EIO);
> *out_len = sizeof(struct fwctl_rpc_cxl_out);
> if (data_size <= 0) {
> rpc_out->size = 0;
> @@ -544,6 +552,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.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
@ 2026-09-04 5:23 ` Alison Schofield
0 siblings, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:23 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:33PM +0800, Richard Cheng wrote:
> 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.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
> Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
> Fixes: 14d502cc2718 ("cxl/mbox: Add SET_FEATURE mailbox command")
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 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.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
2026-09-02 5:38 ` [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
@ 2026-09-04 5:25 ` Alison Schofield
0 siblings, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:25 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:35PM +0800, Richard Cheng wrote:
> 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().
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
> 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.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query
2026-09-02 5:38 ` [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
@ 2026-09-04 5:31 ` Alison Schofield
0 siblings, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:31 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:37PM +0800, Richard Cheng wrote:
> 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.
Before be5cbd084027 pmem was queried first, then ram, so a ram failure
was reported after the pmem records had been collected. Now w the order
reversed, breaking on a ram error means pmem is never queried. I'm not
sure that is what we want.
What matters more to poison list user - a truthful return code or every
record the device is able to give us?
-- Alison
>
> Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Closes: https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=5
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
> 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.53.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
2026-09-02 5:52 ` sashiko-bot
@ 2026-09-04 5:57 ` Alison Schofield
1 sibling, 0 replies; 16+ messages in thread
From: Alison Schofield @ 2026-09-04 5:57 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, dave.jiang, vishal.l.verma, iweiny, ming.li, gourry,
rrichter, linux-cxl, linux-kernel, kees, newtonl, kristinc,
kaihengf, kobak
On Wed, Sep 02, 2026 at 01:38:38PM +0800, Richard Cheng wrote:
> __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;
-ENODEV disables poison listing for the whole memdev since this is a
device_for_each_child() callback.
Before be5cbd084027 that decoder was scanned like any other and the mode was
only ever compared, never used as an index, so CXL_DECODER_NONE and
CXL_DECODER_DEAD both still got their DPA read and the walk continued.
Can the fix be less instrusive and not fail the scan?
Here's a diff, totally untested:
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index afe3fb57b7fe..ebf06edf23be 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2942,9 +2942,9 @@ static int poison_by_decoder(struct device *dev, void *arg)
{
struct cxl_poison_context *ctx = arg;
struct cxl_endpoint_decoder *cxled;
- enum cxl_partition_mode mode;
struct cxl_dev_state *cxlds;
struct cxl_memdev *cxlmd;
+ bool tolerate_efault;
u64 offset, length;
int rc = 0;
@@ -2954,18 +2954,18 @@ 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;
- mode = cxlds->part[cxled->part].mode;
+ /* Without a partition the mode is unknown, do not tolerate -EFAULT */
+ tolerate_efault = cxled->part >= 0 &&
+ cxlds->part[cxled->part].mode == CXL_PARTMODE_RAM;
if (cxled->skip) {
offset = cxled->dpa_res->start - cxled->skip;
length = cxled->skip;
rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
- if (rc == -EFAULT && mode == CXL_PARTMODE_RAM)
+ if (rc == -EFAULT && tolerate_efault)
rc = 0;
if (rc)
return rc;
@@ -2974,7 +2974,7 @@ static int poison_by_decoder(struct device *dev, void *arg)
offset = cxled->dpa_res->start;
length = cxled->dpa_res->end - offset + 1;
rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
- if (rc == -EFAULT && mode == CXL_PARTMODE_RAM)
+ if (rc == -EFAULT && tolerate_efault)
rc = 0;
if (rc)
return rc;
(END)
>
> cxlmd = cxled_to_memdev(cxled);
> cxlds = cxlmd->cxlds;
> --
> 2.53.0
>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
` (7 preceding siblings ...)
2026-09-04 5:20 ` [PATCH v7 0/7] cxl: Sashiko bug fixes Alison Schofield
@ 2026-09-04 16:19 ` Dave Jiang
8 siblings, 0 replies; 16+ messages in thread
From: Dave Jiang @ 2026-09-04 16:19 UTC (permalink / raw)
To: Richard Cheng, dave, jic23, alison.schofield, vishal.l.verma
Cc: iweiny, ming.li, gourry, rrichter, linux-cxl, linux-kernel, kees,
newtonl, kristinc, kaihengf, kobak
On 9/1/26 10:38 PM, Richard Cheng wrote:
> 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.
>
> Patch 7: the Get and Set Feature fwctl handlers converted all helper
> failures into normal RPC responses, sometimes with a SUCCESS device
> status. Propagate delivery failures as ioctl errors while continuing to
> report actual device errors through rpc_out->retval.
>
> A nonzero short Get Feature response is valid when Offset + Count runs
> past the end of the feature. Preserve the returned bytes as a successful
> partial transfer, reject unexpected zero-length success responses, and
> require fixed-format EDAC callers to receive their complete attribute
> structures before consuming them.
>
> Changes since v6 [1]:
> - Patch 7: Remove redundant braces. (Dave Jiang)
>
> [1]:
> https://lore.kernel.org/linux-cxl/20260826014508.9989-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 | 30 +++++++++++++-------
> drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
> drivers/cxl/core/memdev.c | 2 ++
> drivers/cxl/core/region.c | 15 +++++-----
> 5 files changed, 71 insertions(+), 40 deletions(-)
>
>
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
Patches 1 and 7 applied to cxl/next
51493ff66ad6
3eb3376944ac
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-09-04 16:19 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 5:38 [PATCH v7 0/7] cxl: Sashiko bug fixes Richard Cheng
2026-09-02 5:38 ` [PATCH v7 1/7] cxl/features: Reject feature offset that overflows 16-bit field Richard Cheng
2026-09-04 5:23 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 2/7] cxl/region: Scan all partitions for unmapped poison Richard Cheng
2026-09-02 5:38 ` [PATCH v7 3/7] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
2026-09-04 5:25 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 4/7] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
2026-09-02 5:38 ` [PATCH v7 5/7] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
2026-09-04 5:31 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 6/7] cxl/region: Reject poison scan for decoder without a partition Richard Cheng
2026-09-02 5:52 ` sashiko-bot
2026-09-04 5:57 ` Alison Schofield
2026-09-02 5:38 ` [PATCH v7 7/7] cxl/fwctl: Propagate feature RPC delivery errors Richard Cheng
2026-09-04 5:21 ` Alison Schofield
2026-09-04 5:20 ` [PATCH v7 0/7] cxl: Sashiko bug fixes Alison Schofield
2026-09-04 16:19 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox