Re: [PATCH v5 1/7] cxl/features: Reject feature offset that overflows 16-bit field

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The bounds check designed to prevent 16-bit offset wrap-around is off by one, making the final byte of the feature space inaccessible.
--

--- Patch [1]: [PATCH v5 1/7] cxl/features: Reject feature offset that overflows 16-bit field ---
commit 48e82cdf4087a2ffbeb7023fda65d3ff4f9b9c6f
Author: Richard Cheng <[email protected]>

cxl/features: Reject feature offset that overflows 16-bit field

This commit adds bounds checking to prevent 16-bit offset wrap-around in CXL
feature commands. It also changes cxl_get_feature() to return ssize_t to
properly report errors instead of conflating them with a zero-byte result.

> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index ba6d2a5acb74a..8d44ce8294978 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
[ ... ]
> @@ -235,7 +235,10 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
>  	if (!feat_out || !feat_out_size)
> -		return 0;
> +		return -EINVAL;
> +
> +	if (feat_out_size > U16_MAX - offset)
> +		return -EINVAL;

[Severity: Medium]
Does this check contain an off-by-one error that rejects valid accesses to the
last byte of the feature space?

If a request targets the final byte of the 64KB feature space with
offset = 65535 and feat_out_size = 1, this evaluates to 1 > 65535 - 65535
(1 > 0), which is true, incorrectly rejecting the single-byte transfer.

>  
>  	uuid_copy(&pi.uuid, feat_uuid);
>  	pi.selection = selection;
[ ... ]
> @@ -288,6 +291,9 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox,
>  	if (return_code)
>  		*return_code = CXL_MBOX_CMD_RC_INPUT;
>  
> +	if (feat_data_size > U16_MAX - offset)
> +		return -EINVAL;
> +

[Severity: Medium]
Does this bounds check in cxl_set_feature() have the same off-by-one issue
restricting access to the last byte of the feature space?

>  	struct cxl_mbox_set_feat_in *pi __free(kfree) =
>  			kzalloc(cxl_mbox->payload_size, GFP_KERNEL);
>  	if (!pi)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1