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

[Alison Schofield]

On Wed, Jun 24, 2026 at 09:47:37PM +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.
> 
> Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
> Reviewed-by: Kai-Heng Feng <kaihengf@xxxxxxxxxx>
> Reviewed-by: Koba Ko <kobak@xxxxxxxxxx>
> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> ---
> Changelog:
> 
> v1 -> v2:
>     - Drop the reproducer and trim the KASAN splat in the commit message
>     - Sent the reproducer as a regression test in ndctl separately.

This patch itself looks good. Looking at the other bounds checks
Sashiko suggests, I'd rather see this all fixed up in one patch or
patchset, rather than dribble in as multiple patches.

Maybe it all fits into one patch, like this:
cxl/features; Add bounds checking for get/set feature commands
or maybe it works better as a set.

Either way, doing in one swoop would be nice!

-- Alison

> 
> 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)
> 
> base-commit: ef0c9f75a19532d7675384708fc8621e10850104
> -- 
> 2.43.0
>