[PATCH] cxl/features: bound fwctl command payload to the input buffer

[PATCH] cxl/features: bound fwctl command payload to the input buffer

[Zhenhao Wan]

fwctl_cmd_rpc() copies cmd->in_len bytes into inbuf = kvzalloc(cmd->in_len)
and passes inbuf and in_len to ->fw_rpc(). The CXL callback cxlctl_fw_rpc()
ignores in_len and never checks the user-controlled op_size against it.

cxlctl_set_feature() bounds op_size only from below
(op_size <= sizeof(feat_in->hdr)) and then reads op_size - sizeof(hdr)
bytes from feat_in->feat_data via cxl_set_feature(). With a small in_len
and a large op_size the first memcpy() already reads past the
kvzalloc(in_len) buffer; the out-of-bounds bytes are placed in the mailbox
payload and sent to the device, and a large enough op_size can walk into
unmapped memory and oops the kernel. The Get paths pin op_size to a fixed
size but likewise read the input struct without checking in_len.

Reject, at the single dispatch point, any request whose fixed header plus
op_size does not fit in the copied-in buffer. The lower-bound test guards
the subtraction and ensures op_size was copied in before it is read.

Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
---
 drivers/cxl/core/features.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 3435db9ea6b1..a76142b72935 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -649,7 +649,13 @@ static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
 	struct cxl_memdev *cxlmd = fwctl_to_memdev(fwctl_dev);
 	struct cxl_features_state *cxlfs = to_cxlfs(cxlmd->cxlds);
 	const struct fwctl_rpc_cxl *rpc_in = in;
-	u16 opcode = rpc_in->opcode;
+	u16 opcode;
+
+	if (in_len < sizeof(rpc_in->hdr) ||
+	    rpc_in->op_size > in_len - sizeof(rpc_in->hdr))
+		return ERR_PTR(-EINVAL);
+
+	opcode = rpc_in->opcode;
 
 	if (!cxlctl_validate_hw_command(cxlfs, rpc_in, scope, opcode))
 		return ERR_PTR(-EINVAL);

---
base-commit: a48671671df5158a0b8e564cd509e04a090a941b
change-id: 20260620-cxl-fwctl-oob-97adb2864c5c

Best regards,
--  
Zhenhao Wan <whi4ed0g@gmail.com>

Re: [PATCH] cxl/features: bound fwctl command payload to the input buffer

[Jason Gunthorpe]

On Sat, Jun 20, 2026 at 12:33:15PM +0800, Zhenhao Wan wrote:
> fwctl_cmd_rpc() copies cmd->in_len bytes into inbuf = kvzalloc(cmd->in_len)
> and passes inbuf and in_len to ->fw_rpc(). The CXL callback cxlctl_fw_rpc()
> ignores in_len and never checks the user-controlled op_size against it.
> 
> cxlctl_set_feature() bounds op_size only from below
> (op_size <= sizeof(feat_in->hdr)) and then reads op_size - sizeof(hdr)
> bytes from feat_in->feat_data via cxl_set_feature(). With a small in_len
> and a large op_size the first memcpy() already reads past the
> kvzalloc(in_len) buffer; the out-of-bounds bytes are placed in the mailbox
> payload and sent to the device, and a large enough op_size can walk into
> unmapped memory and oops the kernel. The Get paths pin op_size to a fixed
> size but likewise read the input struct without checking in_len.
> 
> Reject, at the single dispatch point, any request whose fixed header plus
> op_size does not fit in the copied-in buffer. The lower-bound test guards
> the subtraction and ensures op_size was copied in before it is read.
> 
> Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
> ---
>  drivers/cxl/core/features.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

CXL folks, you will pick this up?

I think it should be cc stable too

Jason

Re: [PATCH] cxl/features: bound fwctl command payload to the input buffer

[Dave Jiang]

On 7/6/26 8:54 AM, Jason Gunthorpe wrote:
> On Sat, Jun 20, 2026 at 12:33:15PM +0800, Zhenhao Wan wrote:
>> fwctl_cmd_rpc() copies cmd->in_len bytes into inbuf = kvzalloc(cmd->in_len)
>> and passes inbuf and in_len to ->fw_rpc(). The CXL callback cxlctl_fw_rpc()
>> ignores in_len and never checks the user-controlled op_size against it.
>>
>> cxlctl_set_feature() bounds op_size only from below
>> (op_size <= sizeof(feat_in->hdr)) and then reads op_size - sizeof(hdr)
>> bytes from feat_in->feat_data via cxl_set_feature(). With a small in_len
>> and a large op_size the first memcpy() already reads past the
>> kvzalloc(in_len) buffer; the out-of-bounds bytes are placed in the mailbox
>> payload and sent to the device, and a large enough op_size can walk into
>> unmapped memory and oops the kernel. The Get paths pin op_size to a fixed
>> size but likewise read the input struct without checking in_len.
>>
>> Reject, at the single dispatch point, any request whose fixed header plus
>> op_size does not fit in the copied-in buffer. The lower-bound test guards
>> the subtraction and ensures op_size was copied in before it is read.
>>
>> Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
>> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
>> Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>
>> ---
>>  drivers/cxl/core/features.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> CXL folks, you will pick this up?
> 
> I think it should be cc stable too

Thanks for the review Jason. I'll pick it up. I actually had the same fix in a branch locally but haven't got around to post yet. 

> 
> Jason

Re: [PATCH] cxl/features: bound fwctl command payload to the input buffer

[Dave Jiang]

On 6/19/26 9:33 PM, Zhenhao Wan wrote:
> fwctl_cmd_rpc() copies cmd->in_len bytes into inbuf = kvzalloc(cmd->in_len)
> and passes inbuf and in_len to ->fw_rpc(). The CXL callback cxlctl_fw_rpc()
> ignores in_len and never checks the user-controlled op_size against it.
> 
> cxlctl_set_feature() bounds op_size only from below
> (op_size <= sizeof(feat_in->hdr)) and then reads op_size - sizeof(hdr)
> bytes from feat_in->feat_data via cxl_set_feature(). With a small in_len
> and a large op_size the first memcpy() already reads past the
> kvzalloc(in_len) buffer; the out-of-bounds bytes are placed in the mailbox
> payload and sent to the device, and a large enough op_size can walk into
> unmapped memory and oops the kernel. The Get paths pin op_size to a fixed
> size but likewise read the input struct without checking in_len.
> 
> Reject, at the single dispatch point, any request whose fixed header plus
> op_size does not fit in the copied-in buffer. The lower-bound test guards
> the subtraction and ensures op_size was copied in before it is read.
> 
> Fixes: eb5dfcb9e36d ("cxl: Add support to handle user feature commands for set feature")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Signed-off-by: Zhenhao Wan <whi4ed0g@gmail.com>

Applied to cxl/next
f687394af983

> ---
>  drivers/cxl/core/features.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 3435db9ea6b1..a76142b72935 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -649,7 +649,13 @@ static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
>  	struct cxl_memdev *cxlmd = fwctl_to_memdev(fwctl_dev);
>  	struct cxl_features_state *cxlfs = to_cxlfs(cxlmd->cxlds);
>  	const struct fwctl_rpc_cxl *rpc_in = in;
> -	u16 opcode = rpc_in->opcode;
> +	u16 opcode;
> +
> +	if (in_len < sizeof(rpc_in->hdr) ||
> +	    rpc_in->op_size > in_len - sizeof(rpc_in->hdr))
> +		return ERR_PTR(-EINVAL);
> +
> +	opcode = rpc_in->opcode;
>  
>  	if (!cxlctl_validate_hw_command(cxlfs, rpc_in, scope, opcode))
>  		return ERR_PTR(-EINVAL);
> 
> ---
> base-commit: a48671671df5158a0b8e564cd509e04a090a941b
> change-id: 20260620-cxl-fwctl-oob-97adb2864c5c
> 
> Best regards,
> --  
> Zhenhao Wan <whi4ed0g@gmail.com>
>