RE: [PATCH next] cxl: fix return value in cxlctl_validate_set_features()

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Alison Schofield <alison.schofield@intel.com>
>Sent: 28 May 2025 16:23
>To: Shiju Jose <shiju.jose@huawei.com>; Dan Carpenter
><dan.carpenter@linaro.org>
>Cc: Davidlohr Bueso <dave@stgolabs.net>; Jonathan Cameron
><jonathan.cameron@huawei.com>; Dave Jiang <dave.jiang@intel.com>; Vishal
>Verma <vishal.l.verma@intel.com>; Ira Weiny <ira.weiny@intel.com>; Dan
>Williams <dan.j.williams@intel.com>; Li Ming <ming.li@zohomail.com>; Fan Ni
><fan.ni@samsung.com>; linux-cxl@vger.kernel.org; linux-
>kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
>Subject: Re: [PATCH next] cxl: fix return value in cxlctl_validate_set_features()
>
>On Wed, May 28, 2025 at 11:11:41AM +0300, Dan Carpenter wrote:
>> The cxlctl_validate_set_features() function is type bool.  It's
>> supposed to return true for valid requests and false for invalid.
>> However, this error path returns ERR_PTR(-EINVAL) which is true when
>> it was intended to return false.
>
>Shiju - Can you trace this one through and add the impact statement?
>Wondering if this is going to fail gracefully, or badly, further down this path?

Hi Alison,

This is introduced when following fwctl specific code
move out of common  function (use both in fwctl and edac path)
get_support_feature_info() to fwctl specific function
cxlctl_validae_set_feature().
"if (rpc_in->op_size < sizeof(uuid_t))
      return ERR_PTR(-EINVAL);"

This may have an impact on fwctl side if the above check pass.

Thanks,
Shiju

>
>>
>> Fixes: f76e0bbc8bc3 ("cxl: Update prototype of function
>> get_support_feature_info()")
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>>  drivers/cxl/core/features.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
>> index 6f2eae1eb126..7c750599ea69 100644
>> --- a/drivers/cxl/core/features.c
>> +++ b/drivers/cxl/core/features.c
>> @@ -544,7 +544,7 @@ static bool cxlctl_validate_set_features(struct
>cxl_features_state *cxlfs,
>>  	u32 flags;
>>
>>  	if (rpc_in->op_size < sizeof(uuid_t))
>> -		return ERR_PTR(-EINVAL);
>> +		return false;
>>
>>  	feat = cxl_feature_info(cxlfs, &rpc_in->set_feat_in.uuid);
>>  	if (IS_ERR(feat))
>> --
>> 2.47.2
>>