[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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] User-controlled output buffer allocation size `out_len` is used without validation, leading to severe heap out-of-bounds writes or OOPS in `cxlctl_get_feature` and `cxlctl_set_feature`.
--

commit 0b6168cb6d2ecfaa5cb9fc6b814d7c56cade18ef
Author: Zhenhao Wan <whi4ed0g@gmail.com>

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

This commit addresses out-of-bounds reads in CXL feature commands by ensuring
the input buffer size is sufficiently large to hold the fixed header and
payload. It rejects requests that do not fit within the copied-in buffer at a
single dispatch point.

> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> --- 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);

[Severity: Critical]
This is a pre-existing issue, but since we are validating in_len here,
should we also validate *out_len?

In cxlctl_get_feature(), the user-specified *out_len determines the
allocation size but is used without verifying it can hold the structure
header and payload:

drivers/cxl/core/features.c:cxlctl_get_feature() {
    ...
    struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
        kvzalloc(out_size, GFP_KERNEL);
    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);
    *out_len = sizeof(struct fwctl_rpc_cxl_out);
    if (!out_size) {
        rpc_out->size = 0;
    ...
}

If *out_len is 0, kvzalloc(0) returns ZERO_SIZE_PTR, which could lead
to an oops when writing to rpc_out->size. If *out_len is smaller than
sizeof(struct fwctl_rpc_cxl_out) + count, could this result in a heap
out-of-bounds write?

A similar pattern exists in cxlctl_set_feature(), where the fixed
header fields are unconditionally written regardless of the allocation
size:

drivers/cxl/core/features.c:cxlctl_set_feature() {
    ...
    out_size = *out_len;

    struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
        kvzalloc(out_size, GFP_KERNEL);
    if (!rpc_out)
        return ERR_PTR(-ENOMEM);

    rpc_out->size = 0;
    ...
}

> +
> +	opcode = rpc_in->opcode;
>  
>  	if (!cxlctl_validate_hw_command(cxlfs, rpc_in, scope, opcode))
>  		return ERR_PTR(-EINVAL);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@gmail.com?part=1