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

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

[Richard Cheng]

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@nvidia.com>
Reviewed-by: Koba Ko <kobak@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
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.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `out_len` bounds check in `cxlctl_set_feature()` leads to heap out-of-bounds write or `ZERO_SIZE_PTR` dereference panic.
- [High] Missing `in_len` bounds validation across all CXL fwctl handlers allows triggering a kernel panic or reading arbitrary kernel heap memory into CXL devices.
- [High] A malicious or malfunctioning CXL device can trigger a heap buffer overflow in `cxl_get_feature` by returning more data than requested in subsequent iterations.
--

commit 2b1a2543e64d377118a202ab8f46bc2a74fccabb
Author: Richard Cheng <icheng@nvidia.com>

cxl/features: Reject Get Feature count larger than the output buffer

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.

> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 85185af46b72d..9c714ee42a411 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)
>  		return ERR_PTR(-ENOMEM);

[Severity: High]
This isn't a bug introduced by this patch, but it looks like
cxlctl_set_feature() might have a similar missing out_len bounds check.

If a user specifies a very small out_len (e.g., 0) for the fwctl rpc ioctl
with cxlctl_set_feature, could this cause a zero size pointer dereference or
a heap out-of-bounds write?

drivers/cxl/core/features.c:cxlctl_set_feature() {
    ...
    struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
        kvzalloc(out_size, GFP_KERNEL);
    ...
    rpc_out->size = 0;
    ...
}

[Severity: High]
This is a pre-existing issue, but does cxlctl_fw_rpc() need to validate
in_len before dereferencing the input pointer?

If in_len is 0, would reading the opcode cause a kernel panic?

drivers/cxl/core/features.c:cxlctl_fw_rpc() {
    ...
    const struct fwctl_rpc_cxl *rpc_in = in;
    u16 opcode = rpc_in->opcode;
    ...
}

Additionally, in cxlctl_set_feature(), data_size is derived from op_size
rather than the actual in_len:

drivers/cxl/core/features.c:cxlctl_set_feature() {
    ...
    data_size = rpc_in->op_size - sizeof(feat_in->hdr);
    ...
}

Could this lead to an out-of-bounds read of the kernel heap in
cxl_set_feature() if the user provides a maliciously large op_size?

drivers/cxl/core/features.c:cxl_set_feature() {
    ...
    memcpy(pi->feat_data, feat_data + data_sent_size, data_in_size);
    ...
}

[Severity: High]
This is a pre-existing issue, but can a malfunctioning CXL device trigger
a heap buffer overflow in cxl_get_feature()?

The size_out capacity is calculated once before the loop:

drivers/cxl/core/features.c:cxl_get_feature() {
    ...
    size_out = min(feat_out_size, cxl_mbox->payload_size);
    ...
    while (data_to_rd_size > 0) {
        ...
        mbox_cmd.size_out = size_out;
        ...
    }
}

If feat_out_size is larger than the payload size, and the hardware returns
more data than data_to_rd_size in the final iteration, will this overflow the
feat_out allocation since the remaining space at feat_out + data_rcvd_size
might be smaller than size_out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624134737.49166-1-icheng@nvidia.com?part=1

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@nvidia.com>
> Reviewed-by: Koba Ko <kobak@nvidia.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
> 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
> 

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

[Alison Schofield]

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@nvidia.com>
> > Reviewed-by: Koba Ko <kobak@nvidia.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > ---
> > 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@gmail.com/
which looks like it covers one of Sashikos's complaints.

> 
> -- 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
> > 
> 

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

[Richard Cheng]

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@nvidia.com>
> > > Reviewed-by: Koba Ko <kobak@nvidia.com>
> > > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > > ---
> > > 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@gmail.com/
> 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.

--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
> > > 
> > 

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

[Dave Jiang]

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@nvidia.com>
>>>> Reviewed-by: Koba Ko <kobak@nvidia.com>
>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>> Signed-off-by: Richard Cheng <icheng@nvidia.com>
>>>> ---
>>>> 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@gmail.com/
>> 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

> 
> --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
>>>>
>>>

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

[Richard Cheng]

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@nvidia.com>
> >>>> Reviewed-by: Koba Ko <kobak@nvidia.com>
> >>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >>>> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> >>>> ---
> >>>> 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@gmail.com/
> >> 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@nvidia.com/

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.

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
> >>>>
> >>>
> 

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@nvidia.com>
>>>>>> Reviewed-by: Koba Ko <kobak@nvidia.com>
>>>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>>>> Signed-off-by: Richard Cheng <icheng@nvidia.com>
>>>>>> ---
>>>>>> 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@gmail.com/
>>>> 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@nvidia.com/
> 
> 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
>>>>>>
>>>>>
>>