Re: [PATCH v4 1/6] 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:

Pre-existing issues:
- [High] fwctl ioctls falsely report SUCCESS to userspace when CXL mailbox transport encounters an internal error.
--

--- Patch [1]: [PATCH v4 1/6] cxl/features: Reject feature offset that overflows 16-bit field ---
commit 758deff635a1582bb94f5fac18c817ddd4966bdb
Author: Richard Cheng <[email protected]>

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

This patch validates that the transfer size does not exceed the remaining 16-bit
feature range to prevent truncation when building mailbox command offsets. It
also changes cxl_get_feature() to return ssize_t so invalid input and mailbox
failures are reported as negative errno. Finally, it updates the EDAC callers
to handle negative results.

> 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
[ ... ]
> @@ -259,7 +262,7 @@ size_t cxl_get_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  		if (rc < 0 || !mbox_cmd.size_out) {
>  			if (return_code)
>  				*return_code = mbox_cmd.return_code;
> -			return 0;
> +			return rc < 0 ? rc : -EIO;
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but if cxl_internal_send_cmd()
fails with an OS-level error (rc < 0), it won't populate
mbox_cmd.return_code. Since mbox_cmd was initialized as a compound literal,
doesn't this set *return_code to 0 (CXL_MBOX_CMD_RC_SUCCESS)?

When this happens, cxlctl_get_feature() will absorb the negative error code
and pass the zeroed return_code back to fwctl:

drivers/cxl/core/features.c:cxlctl_get_feature() {
	...
	data_size = cxl_get_feature(..., &return_code);
	*out_len = sizeof(struct fwctl_rpc_cxl_out);
	if (data_size <= 0) {
		rpc_out->size = 0;
		rpc_out->retval = return_code;
		return no_free_ptr(rpc_out);
	}
	...
}

Does this result in fwctl ioctls falsely reporting SUCCESS to userspace when
the CXL mailbox transport encounters an internal error? This same pattern
appears to be present in cxl_set_feature() and cxlctl_set_feature() as well.

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