* [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer
2026-06-26 10:40 [PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands Richard Cheng
@ 2026-06-26 10:41 ` Richard Cheng
2026-06-26 10:54 ` sashiko-bot
2026-06-26 10:41 ` [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header Richard Cheng
2026-06-26 10:41 ` [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer Richard Cheng
2 siblings, 1 reply; 10+ messages in thread
From: Richard Cheng @ 2026-06-26 10:41 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, djbw,
danwilliams
Cc: iweiny, ming.li, kobak, kaihengf, kees, newtonl, kristinc, mochs,
linux-cxl, linux-kernel, Richard Cheng
cxlctl_get_feature() sizes its output buffer from the user's
fwctl_rpc.out_len, but the device is told to write
cxl_mbox_get_feat_in.count bytes into rpc_out->payload, which is a
separate user-controlled value. Nothing bounds count against out_len, so
a small out_len with a large count overflows the kvzalloc()'d buffer.
A heap OOB write reachable from FWCTL_RPC.
Reject requests where count exceeds the available payload room, before
allocating.
Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
Reviewed-by: Kai-Heng Feng <kaihengf@nvidia.com>
Reviewed-by: Koba Ko <kobak@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:
v2 -> v3:
- No change.
v1 -> v2:
- Drop the reproducer and trim the KASAN splat in the commit message
- Sent the reproducer as a regression test in ndctl separately.
Best regards,
Richard Cheng
---
drivers/cxl/core/features.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 85185af46b72..9c714ee42a41 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
if (!count)
return ERR_PTR(-EINVAL);
+ if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
+ count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
+ return ERR_PTR(-EINVAL);
+
struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
kvzalloc(out_size, GFP_KERNEL);
if (!rpc_out)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer
2026-06-26 10:41 ` [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer Richard Cheng
@ 2026-06-26 10:54 ` sashiko-bot
2026-06-30 2:50 ` Richard Cheng
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-06-26 10:54 UTC (permalink / raw)
To: Richard Cheng; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing bounds checks on user-provided input length in `cxlctl_fw_rpc()` and `cxlctl_set_feature()`, leading to out-of-bounds reads and potential ZERO_SIZE_PTR crashes.
--
commit 1f7098b182e4cdaf480b19545ca818cbf043bd29
Author: Richard Cheng <icheng@nvidia.com>
cxl/features: Reject Get Feature count larger than the output buffer
cxlctl_get_feature() sizes its output buffer from the user's
fwctl_rpc.out_len, but the device is told to write count bytes into
rpc_out->payload. This adds bounds checking against out_size to prevent
overflowing the allocated buffer.
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 85185af46b72d..9c714ee42a411 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
> if (!count)
> return ERR_PTR(-EINVAL);
>
> + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
> + count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
> + return ERR_PTR(-EINVAL);
> +
[Severity: Critical]
This is a pre-existing issue, but while we are adding bounds checks for
the output buffer here, does cxlctl_fw_rpc() also need to validate the
user-provided in_len?
If the ioctl is called with an in_len of 0, could dereferencing
rpc_in->opcode in cxlctl_fw_rpc() lead to a zero-size pointer crash?
Similarly, in cxlctl_set_feature(), the operation size is calculated without
verifying in_len:
data_size = rpc_in->op_size - sizeof(feat_in->hdr);
Could a small in_len combined with a large rpc_in->op_size result in an
out-of-bounds read, copying adjacent kernel heap memory into the hardware
mailbox? Would validating in_len against op_size in cxlctl_fw_rpc() prevent
this?
> struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> kvzalloc(out_size, GFP_KERNEL);
> if (!rpc_out)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer
2026-06-26 10:54 ` sashiko-bot
@ 2026-06-30 2:50 ` Richard Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Richard Cheng @ 2026-06-30 2:50 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-cxl
On Fri, Jun 26, 2026 at 10:54:07AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing bounds checks on user-provided input length in `cxlctl_fw_rpc()` and `cxlctl_set_feature()`, leading to out-of-bounds reads and potential ZERO_SIZE_PTR crashes.
> --
>
> commit 1f7098b182e4cdaf480b19545ca818cbf043bd29
> Author: Richard Cheng <icheng@nvidia.com>
>
> cxl/features: Reject Get Feature count larger than the output buffer
>
> cxlctl_get_feature() sizes its output buffer from the user's
> fwctl_rpc.out_len, but the device is told to write count bytes into
> rpc_out->payload. This adds bounds checking against out_size to prevent
> overflowing the allocated buffer.
>
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index 85185af46b72d..9c714ee42a411 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> > @@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
> > if (!count)
> > return ERR_PTR(-EINVAL);
> >
> > + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
> > + count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
> > + return ERR_PTR(-EINVAL);
> > +
>
> [Severity: Critical]
> This is a pre-existing issue, but while we are adding bounds checks for
> the output buffer here, does cxlctl_fw_rpc() also need to validate the
> user-provided in_len?
>
> If the ioctl is called with an in_len of 0, could dereferencing
> rpc_in->opcode in cxlctl_fw_rpc() lead to a zero-size pointer crash?
>
> Similarly, in cxlctl_set_feature(), the operation size is calculated without
> verifying in_len:
>
> data_size = rpc_in->op_size - sizeof(feat_in->hdr);
>
> Could a small in_len combined with a large rpc_in->op_size result in an
> out-of-bounds read, copying adjacent kernel heap memory into the hardware
> mailbox? Would validating in_len against op_size in cxlctl_fw_rpc() prevent
> this?
>
> > struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> > kvzalloc(out_size, GFP_KERNEL);
> > if (!rpc_out)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=1
I think this is addressed here.
https://lore.kernel.org/linux-cxl/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/
--Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header
2026-06-26 10:40 [PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands Richard Cheng
2026-06-26 10:41 ` [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer Richard Cheng
@ 2026-06-26 10:41 ` Richard Cheng
2026-06-26 10:56 ` sashiko-bot
2026-06-26 10:41 ` [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer Richard Cheng
2 siblings, 1 reply; 10+ messages in thread
From: Richard Cheng @ 2026-06-26 10:41 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, djbw,
danwilliams
Cc: iweiny, ming.li, kobak, kaihengf, kees, newtonl, kristinc, mochs,
linux-cxl, linux-kernel, Richard Cheng
cxlctl_set_feature() sizes its output buffer from the user's
fwctl_rpc.out_len but never checks it is large enough to hold even the
fwctl_rpc_cxl_out header. With out_len == 0 , kvzalloc() returns
ZERO_SIZE_PTR, which passes the !rpc_out check, the subsequent
rpc_out->size = 0 then writes through the poison pointer.
Reject requests whose output buffer can't hold the response header,
before allocating. The Set Feature reply carries no payload, so the
header is all that is required.
Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:
v2 -> v3:
- New patch.
drivers/cxl/core/features.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 9c714ee42a41..ed18ccb5e236 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -520,6 +520,9 @@ static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
flags = le32_to_cpu(feat_in->flags);
out_size = *out_len;
+ if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload))
+ return ERR_PTR(-EINVAL);
+
struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
kvzalloc(out_size, GFP_KERNEL);
if (!rpc_out)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header
2026-06-26 10:41 ` [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header Richard Cheng
@ 2026-06-26 10:56 ` sashiko-bot
2026-06-30 2:51 ` Richard Cheng
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-06-26 10:56 UTC (permalink / raw)
To: Richard Cheng; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing validation of the user-provided `in_len` parameter allows a `ZERO_SIZE_PTR` dereference and severe out-of-bounds reads in the CXL FWCTL driver.
--
commit 149380dfe9472c578ca9fd76d4e5bb0c5ac85c19
Author: Richard Cheng <icheng@nvidia.com>
cxl/features: Reject Set Features output buffer smaller than the header
cxlctl_set_feature() sizes its output buffer from the user's
fwctl_rpc.out_len but never checks it is large enough to hold even the
fwctl_rpc_cxl_out header. With out_len == 0 , kvzalloc() returns
ZERO_SIZE_PTR, which passes the !rpc_out check, the subsequent
rpc_out->size = 0 then writes through the poison pointer.
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 9c714ee42a411..ed18ccb5e236e 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -520,6 +520,9 @@ static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
> flags = le32_to_cpu(feat_in->flags);
> out_size = *out_len;
>
> + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload))
> + return ERR_PTR(-EINVAL);
> +
[Severity: Critical]
This is a pre-existing issue, but missing validation of the user-provided
in_len parameter allows a ZERO_SIZE_PTR dereference and out-of-bounds reads.
If a malicious user passes in_len=0 to the FWCTL_RPC ioctl, kvzalloc(0)
returns the ZERO_SIZE_PTR constant (0x10) in
drivers/fwctl/main.c:fwctl_cmd_rpc():
void *inbuf __free(kvfree) = kvzalloc(cmd->in_len, GFP_KERNEL_ACCOUNT);
The driver unconditionally dereferences this pointer in
drivers/cxl/core/features.c:cxlctl_fw_rpc():
const struct fwctl_rpc_cxl *rpc_in = in;
u16 opcode = rpc_in->opcode;
Also, if a user provides a small in_len but specifies a massive op_size in
the command header, handlers like cxlctl_set_feature() will read out of
bounds of the kvzalloc'd heap buffer and transmit that data to the hardware
mailbox:
data_size = rpc_in->op_size - sizeof(feat_in->hdr);
rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
feat_in->version, feat_in->feat_data,
data_size, flags, offset, &return_code);
Should there be validation of in_len before casting and accessing the input
buffer, as well as a check to ensure rpc_in->op_size does not exceed in_len?
> struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> kvzalloc(out_size, GFP_KERNEL);
> if (!rpc_out)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header
2026-06-26 10:56 ` sashiko-bot
@ 2026-06-30 2:51 ` Richard Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Richard Cheng @ 2026-06-30 2:51 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-cxl
On Fri, Jun 26, 2026 at 10:56:11AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Missing validation of the user-provided `in_len` parameter allows a `ZERO_SIZE_PTR` dereference and severe out-of-bounds reads in the CXL FWCTL driver.
> --
>
> commit 149380dfe9472c578ca9fd76d4e5bb0c5ac85c19
> Author: Richard Cheng <icheng@nvidia.com>
>
> cxl/features: Reject Set Features output buffer smaller than the header
>
> cxlctl_set_feature() sizes its output buffer from the user's
> fwctl_rpc.out_len but never checks it is large enough to hold even the
> fwctl_rpc_cxl_out header. With out_len == 0 , kvzalloc() returns
> ZERO_SIZE_PTR, which passes the !rpc_out check, the subsequent
> rpc_out->size = 0 then writes through the poison pointer.
>
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index 9c714ee42a411..ed18ccb5e236e 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> > @@ -520,6 +520,9 @@ static void *cxlctl_set_feature(struct cxl_features_state *cxlfs,
> > flags = le32_to_cpu(feat_in->flags);
> > out_size = *out_len;
> >
> > + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload))
> > + return ERR_PTR(-EINVAL);
> > +
>
> [Severity: Critical]
> This is a pre-existing issue, but missing validation of the user-provided
> in_len parameter allows a ZERO_SIZE_PTR dereference and out-of-bounds reads.
>
> If a malicious user passes in_len=0 to the FWCTL_RPC ioctl, kvzalloc(0)
> returns the ZERO_SIZE_PTR constant (0x10) in
> drivers/fwctl/main.c:fwctl_cmd_rpc():
>
> void *inbuf __free(kvfree) = kvzalloc(cmd->in_len, GFP_KERNEL_ACCOUNT);
>
> The driver unconditionally dereferences this pointer in
> drivers/cxl/core/features.c:cxlctl_fw_rpc():
>
> const struct fwctl_rpc_cxl *rpc_in = in;
> u16 opcode = rpc_in->opcode;
>
> Also, if a user provides a small in_len but specifies a massive op_size in
> the command header, handlers like cxlctl_set_feature() will read out of
> bounds of the kvzalloc'd heap buffer and transmit that data to the hardware
> mailbox:
>
> data_size = rpc_in->op_size - sizeof(feat_in->hdr);
> rc = cxl_set_feature(cxl_mbox, &feat_in->uuid,
> feat_in->version, feat_in->feat_data,
> data_size, flags, offset, &return_code);
>
> Should there be validation of in_len before casting and accessing the input
> buffer, as well as a check to ensure rpc_in->op_size does not exceed in_len?
>
> > struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> > kvzalloc(out_size, GFP_KERNEL);
> > if (!rpc_out)
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=2
Same as the previous one,
addressed here
https://lore.kernel.org/linux-cxl/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/
--Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer
2026-06-26 10:40 [PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands Richard Cheng
2026-06-26 10:41 ` [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer Richard Cheng
2026-06-26 10:41 ` [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header Richard Cheng
@ 2026-06-26 10:41 ` Richard Cheng
2026-06-26 10:52 ` sashiko-bot
2 siblings, 1 reply; 10+ messages in thread
From: Richard Cheng @ 2026-06-26 10:41 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, djbw,
danwilliams
Cc: iweiny, ming.li, kobak, kaihengf, kees, newtonl, kristinc, mochs,
linux-cxl, linux-kernel, Richard Cheng
cxl_get_feature() reads a feature in a loop but passes a fixed size_out
as the output capacity every iteration. On the last partial iteration
the buffer has less room left, so a device that returns more than asked
can overflow feat_out.
Use the per-iter size data_to_rd_size, which already tracks the
remaining room, as the output capacity.
Fixes: 5e5ac21f629d ("cxl/mbox: Add GET_FEATURE mailbox command")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:
v2 -> v3:
- New patch.
drivers/cxl/core/features.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index ed18ccb5e236..e52371f87300 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -225,7 +225,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
void *feat_out, size_t feat_out_size, u16 offset,
u16 *return_code)
{
- size_t data_to_rd_size, size_out;
+ size_t data_to_rd_size;
struct cxl_mbox_get_feat_in pi;
struct cxl_mbox_cmd mbox_cmd;
size_t data_rcvd_size = 0;
@@ -237,7 +237,6 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
if (!feat_out || !feat_out_size)
return 0;
- size_out = min(feat_out_size, cxl_mbox->payload_size);
uuid_copy(&pi.uuid, feat_uuid);
pi.selection = selection;
do {
@@ -250,7 +249,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
.opcode = CXL_MBOX_OP_GET_FEATURE,
.size_in = sizeof(pi),
.payload_in = &pi,
- .size_out = size_out,
+ .size_out = data_to_rd_size,
.payload_out = feat_out + data_rcvd_size,
.min_out = data_to_rd_size,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer
2026-06-26 10:41 ` [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer Richard Cheng
@ 2026-06-26 10:52 ` sashiko-bot
2026-06-30 2:57 ` Richard Cheng
0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-06-26 10:52 UTC (permalink / raw)
To: Richard Cheng; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Silent 16-bit offset wrap-around in CXL feature commands allows device state corruption.
--
commit 55606972e1fe115ff716307a2dad0a662361b0c3
Author: Richard Cheng <icheng@nvidia.com>
cxl/features: Clamp Get Feature output size to the remaining buffer
cxl_get_feature() reads a feature in a loop but passes a fixed size_out
as the output capacity every iteration. This commit changes size_out
to use the remaining per-iter size, preventing a device from overflowing
feat_out on the last partial iteration.
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index ed18ccb5e236e..e52371f873006 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
[ ... ]
> @@ -237,7 +237,6 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> if (!feat_out || !feat_out_size)
> return 0;
>
> - size_out = min(feat_out_size, cxl_mbox->payload_size);
> uuid_copy(&pi.uuid, feat_uuid);
> pi.selection = selection;
> do {
> @@ -250,7 +249,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
[Severity: High]
This is a pre-existing issue, but does pi.offset silently overflow during
the loop if offset + data_rcvd_size exceeds 65535?
Looking at cxl_get_feature(), pi.offset is updated right before this
struct initialization:
pi.offset = cpu_to_le16(offset + data_rcvd_size);
Here offset is a u16 and data_rcvd_size is a size_t, but pi.offset
is __le16. If a user requests a large operation through the fwctl
interface that sums to greater than 65535, the offset will wrap around and
truncate the value.
Could this cause the device to read from (or in the case of
cxl_set_feature(), write to) the wrong offset, silently corrupting hardware
feature data?
> .opcode = CXL_MBOX_OP_GET_FEATURE,
> .size_in = sizeof(pi),
> .payload_in = &pi,
> - .size_out = size_out,
> + .size_out = data_to_rd_size,
> .payload_out = feat_out + data_rcvd_size,
> .min_out = data_to_rd_size,
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer
2026-06-26 10:52 ` sashiko-bot
@ 2026-06-30 2:57 ` Richard Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Richard Cheng @ 2026-06-30 2:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-cxl
On Fri, Jun 26, 2026 at 10:52:55AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Silent 16-bit offset wrap-around in CXL feature commands allows device state corruption.
> --
>
> commit 55606972e1fe115ff716307a2dad0a662361b0c3
> Author: Richard Cheng <icheng@nvidia.com>
>
> cxl/features: Clamp Get Feature output size to the remaining buffer
>
> cxl_get_feature() reads a feature in a loop but passes a fixed size_out
> as the output capacity every iteration. This commit changes size_out
> to use the remaining per-iter size, preventing a device from overflowing
> feat_out on the last partial iteration.
>
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index ed18ccb5e236e..e52371f873006 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> [ ... ]
> > @@ -237,7 +237,6 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
> > if (!feat_out || !feat_out_size)
> > return 0;
> >
> > - size_out = min(feat_out_size, cxl_mbox->payload_size);
> > uuid_copy(&pi.uuid, feat_uuid);
> > pi.selection = selection;
> > do {
> > @@ -250,7 +249,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>
> [Severity: High]
> This is a pre-existing issue, but does pi.offset silently overflow during
> the loop if offset + data_rcvd_size exceeds 65535?
>
> Looking at cxl_get_feature(), pi.offset is updated right before this
> struct initialization:
>
> pi.offset = cpu_to_le16(offset + data_rcvd_size);
>
> Here offset is a u16 and data_rcvd_size is a size_t, but pi.offset
> is __le16. If a user requests a large operation through the fwctl
> interface that sums to greater than 65535, the offset will wrap around and
> truncate the value.
>
> Could this cause the device to read from (or in the case of
> cxl_set_feature(), write to) the wrong offset, silently corrupting hardware
> feature data?
>
> > .opcode = CXL_MBOX_OP_GET_FEATURE,
> > .size_in = sizeof(pi),
> > .payload_in = &pi,
> > - .size_out = size_out,
> > + .size_out = data_to_rd_size,
> > .payload_out = feat_out + data_rcvd_size,
> > .min_out = data_to_rd_size,
> > };
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260626104102.53892-1-icheng@nvidia.com?part=3
This is a different one , I'll include it in another series.
--Richard
^ permalink raw reply [flat|nested] 10+ messages in thread