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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `out_len` bounds check in `cxlctl_set_feature()` leads to heap out-of-bounds write or `ZERO_SIZE_PTR` dereference panic.
- [High] Missing `in_len` bounds validation across all CXL fwctl handlers allows triggering a kernel panic or reading arbitrary kernel heap memory into CXL devices.
- [High] A malicious or malfunctioning CXL device can trigger a heap buffer overflow in `cxl_get_feature` by returning more data than requested in subsequent iterations.
--

commit 2b1a2543e64d377118a202ab8f46bc2a74fccabb
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
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.

> 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);
> +
>  	struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
>  		kvzalloc(out_size, GFP_KERNEL);
>  	if (!rpc_out)
>  		return ERR_PTR(-ENOMEM);

[Severity: High]
This isn't a bug introduced by this patch, but it looks like
cxlctl_set_feature() might have a similar missing out_len bounds check.

If a user specifies a very small out_len (e.g., 0) for the fwctl rpc ioctl
with cxlctl_set_feature, could this cause a zero size pointer dereference or
a heap out-of-bounds write?

drivers/cxl/core/features.c:cxlctl_set_feature() {
    ...
    struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
        kvzalloc(out_size, GFP_KERNEL);
    ...
    rpc_out->size = 0;
    ...
}

[Severity: High]
This is a pre-existing issue, but does cxlctl_fw_rpc() need to validate
in_len before dereferencing the input pointer?

If in_len is 0, would reading the opcode cause a kernel panic?

drivers/cxl/core/features.c:cxlctl_fw_rpc() {
    ...
    const struct fwctl_rpc_cxl *rpc_in = in;
    u16 opcode = rpc_in->opcode;
    ...
}

Additionally, in cxlctl_set_feature(), data_size is derived from op_size
rather than the actual in_len:

drivers/cxl/core/features.c:cxlctl_set_feature() {
    ...
    data_size = rpc_in->op_size - sizeof(feat_in->hdr);
    ...
}

Could this lead to an out-of-bounds read of the kernel heap in
cxl_set_feature() if the user provides a maliciously large op_size?

drivers/cxl/core/features.c:cxl_set_feature() {
    ...
    memcpy(pi->feat_data, feat_data + data_sent_size, data_in_size);
    ...
}

[Severity: High]
This is a pre-existing issue, but can a malfunctioning CXL device trigger
a heap buffer overflow in cxl_get_feature()?

The size_out capacity is calculated once before the loop:

drivers/cxl/core/features.c:cxl_get_feature() {
    ...
    size_out = min(feat_out_size, cxl_mbox->payload_size);
    ...
    while (data_to_rd_size > 0) {
        ...
        mbox_cmd.size_out = size_out;
        ...
    }
}

If feat_out_size is larger than the payload size, and the hardware returns
more data than data_to_rd_size in the final iteration, will this overflow the
feat_out allocation since the remaining space at feat_out + data_rcvd_size
might be smaller than size_out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624134737.49166-1-icheng@nvidia.com?part=1