[PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands

[PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands

[Richard Cheng]

The CXL fwctl feature handlers take buffer sizes from userspace, which is
out_len, and from the device without fully checking them. This series
adds the missing bounds checks.

Patch 1: reject a Get Feature whose count is larger than the output
         buffer.
Patch 2: reject a Set Feature whose output buffer is too small for the
         reply header. A zero out_len makes kvzalloc() return
         ZERO_SIZE_PTR, and the header write then corrupts memory.
Patch 3: clamp the Get Feature read loop to the room left in the output
         buffer, so a device that returns more than requested cannot
         write past it.

A related gap is fixed separately by Zhenhao Wan's patch [1].

Changes since v2:
    - Expand the single Get Feature fix into a series that also covers
      the Set Feature output buffer and the Get Feature read loop.

[1]:
https://lore.kernel.org/all/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/

Richard Cheng (3):
  cxl/features: Reject Get Feature count larger than the output buffer
  cxl/features: Reject Set Features output buffer smaller than the
    header
  cxl/features: Clamp Get Feature output size to the remaining buffer

 drivers/cxl/core/features.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)


base-commit: ef0c9f75a19532d7675384708fc8621e10850104
-- 
2.43.0

[PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer

[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

[PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header

[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

[PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer

[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

Re: [PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands

[Richard Cheng]

On Fri, Jun 26, 2026 at 06:40:59PM +0800, Richard Cheng wrote:
> The CXL fwctl feature handlers take buffer sizes from userspace, which is
> out_len, and from the device without fully checking them. This series
> adds the missing bounds checks.
> 
> Patch 1: reject a Get Feature whose count is larger than the output
>          buffer.
> Patch 2: reject a Set Feature whose output buffer is too small for the
>          reply header. A zero out_len makes kvzalloc() return
>          ZERO_SIZE_PTR, and the header write then corrupts memory.
> Patch 3: clamp the Get Feature read loop to the room left in the output
>          buffer, so a device that returns more than requested cannot
>          write past it.
> 
> A related gap is fixed separately by Zhenhao Wan's patch [1].
> 
> Changes since v2:
>     - Expand the single Get Feature fix into a series that also covers
>       the Set Feature output buffer and the Get Feature read loop.
> 
> [1]:
> https://lore.kernel.org/all/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/
>

Hello,

Just a gentle ping on this serie.

As https://lore.kernel.org/linux-cxl/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/

landed , I think these are worth to be taken a look as well?

Best regards,
Richard Cheng.
 
> Richard Cheng (3):
>   cxl/features: Reject Get Feature count larger than the output buffer
>   cxl/features: Reject Set Features output buffer smaller than the
>     header
>   cxl/features: Clamp Get Feature output size to the remaining buffer
> 
>  drivers/cxl/core/features.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> 
> base-commit: ef0c9f75a19532d7675384708fc8621e10850104
> -- 
> 2.43.0
> 

Re: [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header

[Dave Jiang]

On 6/26/26 3:41 AM, Richard Cheng wrote:
> 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>

Reviewed-by: Dave Jiang <dave.jiang@intel.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)

Re: [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer

[Dave Jiang]

On 6/26/26 3:41 AM, Richard Cheng wrote:
> 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>

Reviewed-by: Dave Jiang <dave.jiang@intel.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,
>  		};

Re: [PATCH v3 1/3] cxl/features: Reject Get Feature count larger than the output buffer

[Alison Schofield]

On Fri, Jun 26, 2026 at 06:41:00PM +0800, Richard Cheng wrote:
> 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.


Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH v3 2/3] cxl/features: Reject Set Features output buffer smaller than the header

[Alison Schofield]

On Fri, Jun 26, 2026 at 06:41:01PM +0800, Richard Cheng wrote:
> 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.

See my message on the ndctl PATCH wrt adding a test case for this one.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH v3 3/3] cxl/features: Clamp Get Feature output size to the remaining buffer

[Alison Schofield]

On Fri, Jun 26, 2026 at 06:41:02PM +0800, Richard Cheng wrote:
> 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.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH v3 0/3] cxl/features: Bounds-check the fwctl feature commands

[Dave Jiang]

On 6/26/26 3:40 AM, Richard Cheng wrote:
> The CXL fwctl feature handlers take buffer sizes from userspace, which is
> out_len, and from the device without fully checking them. This series
> adds the missing bounds checks.
> 
> Patch 1: reject a Get Feature whose count is larger than the output
>          buffer.
> Patch 2: reject a Set Feature whose output buffer is too small for the
>          reply header. A zero out_len makes kvzalloc() return
>          ZERO_SIZE_PTR, and the header write then corrupts memory.
> Patch 3: clamp the Get Feature read loop to the room left in the output
>          buffer, so a device that returns more than requested cannot
>          write past it.
> 
> A related gap is fixed separately by Zhenhao Wan's patch [1].
> 
> Changes since v2:
>     - Expand the single Get Feature fix into a series that also covers
>       the Set Feature output buffer and the Get Feature read loop.
> 
> [1]:
> https://lore.kernel.org/all/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com/
> 
> Richard Cheng (3):
>   cxl/features: Reject Get Feature count larger than the output buffer
>   cxl/features: Reject Set Features output buffer smaller than the
>     header
>   cxl/features: Clamp Get Feature output size to the remaining buffer
> 
>  drivers/cxl/core/features.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> 
> base-commit: ef0c9f75a19532d7675384708fc8621e10850104

Applied to cxl/next
2aeb21fe557e cxl/features: Clamp Get Feature output size to the remaining buffer
cde18d6c1d91 cxl/features: Reject Set Features output buffer smaller than the header
4bf6bac37507 cxl/features: Reject Get Feature count larger than the output buffer

There was a conflict for 2aeb21fe557e. May want to check that it applied correctly.