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

[Dave Jiang]

On 6/28/26 7:12 PM, Richard Cheng wrote:
> On Fri, Jun 26, 2026 at 09:17:03AM +0800, Dave Jiang wrote:
>>
>>
>> On 6/26/26 12:27 AM, Richard Cheng wrote:
>>> On Wed, Jun 24, 2026 at 02:10:26PM +0800, Alison Schofield wrote:
>>>> On Wed, Jun 24, 2026 at 01:54:50PM -0700, Alison Schofield wrote:
>>>>> 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!
>>>>
>>>> Oh, seems I'm reading patches out of order.
>>>>
>>>> Now I see this: 
>>>> https://lore.kernel.org/linux-cxl/20260620-cxl-fwctl-oob-v1-1-5758e34d784a@xxxxxxxxx/
>>>> which looks like it covers one of Sashikos's complaints.
>>>>
>>>
>>> Hi Alison,
>>>
>>> Thanks for pointing this out.
>>>
>>> Then that would be no problem for me, I'll append my fixes with Zhenhao's patch.
>>>
>>> so my original fix + sashiko's complain about cxlctl_set_feature() and cxl_get_feature().
>>> I'll put them in my work and send as a serie.
>>
>> Hi Richard, I see sashiko pointed out some existing issues on the series. Will you create fixes for them or should I go do that?
>>
>> DJ
>>
> 
> Hi Dave,
> 
> I've sent out v3 of this here.
> 
> https://lore.kernel.org/all/20260626104102.53892-1-icheng@xxxxxxxxxx/
> 
> I've made the fixes of the bug sashiko-bot pointed out here in that serie.
> 
> I also see it's mentioning more pre-existing bugs on v3, I'll do them in seperate patches,
> or are you refering to the issue in this one ? if you're talking about the latter, I've
> included them in v3.


I'm talking about the complaint of pre-existing issues.

DJ

> 
> Please take a look while you're available, thanks.
> 
> Best regards,
> Richard Cheng.
> 
>  
>>>
>>> --Richard
>>>  
>>>>>
>>>>> -- 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
>>>>>>
>>>>>
>>