Re: [PATCH RESEND] cxl/mbox: validate the DCD extent list counts against the payload

[Anisa Su]

On Thu, Sep 17, 2026 at 06:46:56PM +0800, Gaobin Huang wrote:
> __cxl_process_extent_list() trusts two counts from the Get DC Extent List
> response: returned_extent_count bounds the loop over the flexible
> extents->extent[] array, and total_extent_count decides when the enclosing
> do/while is finished.  Neither is compared against what the device
> actually returned, and the command is only issued with .min_out = 1.
> 
> A device that reports more extents than it delivered walks the loop past
> the end of the mailbox buffer.  A device that reports a large total while
> returning nothing makes the loop spin forever, because total_read never
> reaches total_expected; a stable generation number and total also keep the
> existing -EAGAIN check from firing.  The caller cannot recover from that
> one: __cxl_process_extent_list() never returns, so the retry loop around
> it never runs.
> 
> Derive the bound from mbox_cmd.size_out, clamp the claim to it, and fail
> with -EIO when the device stops making progress.  min_out is smaller than
> the response header, so the subtraction needs the same underflow guard.
> 
Hello Gaobin,

As Jonathan mentioned in his reply, specific debug info does not need to be
included in the commit message. I would suggest adding this kind of information
to a cover letter.

> Seen with QEMU emulating a device that lies.  The response header is 16
> bytes and an extent is 40, so a 2048 byte mailbox buffer holds 50:
> 
> 	BUG: KASAN: slab-out-of-bounds in cxl_validate_extent+0xca/0x310
> 	Read of size 2 at addr ffff888005758800 by task sh/1
> 	 cxl_validate_extent+0xca/0x310
> 	 cxl_process_extent_list+0x2c1/0x430
> 	 cxl_region_probe+0xb2b/0xc40
> 	 which belongs to the cache kmalloc-2k of size 2048
> 
> The Read of size 2 is extent->shared_extn_seq.  extent[50] starts at
> 16 + 50 * 40 = 2016 and the field is 32 bytes into the record, so that
> read is the first byte outside the buffer; a claimed count of 50 stays
> inside and 51 does not, as the sweep shows.  Reporting
> total_extent_count = 100000 with returned_extent_count = 0 instead spins
> region bring-up until the guest stops answering console commands; with the
> fix it logs "Extent list: no progress after 0/100000" and fails.
> 
> This is in the dynamic capacity device series under review (branch
> dcd-v6-2025-04-13 of weiny2/linux-kernel, based on 6.15-rc2), so the fix
> belongs in that series before it is merged.
> 

Ira has dropped the DCD series and I have continued to work on it. The last
revision is here:
https://lore.kernel.org/linux-cxl/20260625112638.550691-1-anisa.su@xxxxxxxxxxx/

Sashiko reported the same issue on the previous revision:
https://lore.kernel.org/linux-cxl/20260625183203.3042C1F000E9@xxxxxxxxxxxxxxx/
and I have already patched it in my working tree.

I plan to post the next revision after the DCD Prep Series is complete:
https://lore.kernel.org/linux-cxl/20260918203049.7273-1-anisa.su@xxxxxxxxxxx/T/#t

You are welcome to review both. FYI, if you would like to keep up-to-date with
community news, there is a monthly meeting at 11AM PST every 3rd Tuesday of the
month. So the next one is Tuesday October 20 11AM PST. If you would like to
attend, I can forward the invite to you.

> Signed-off-by: Gaobin Huang <huanggaobin23@xxxxxxxxxx>
> ---
> Resend note: the patch is byte-for-byte unchanged.  v1 went to
> ira.weiny@xxxxxxxxx, which no longer exists, so the author it is addressed to
> never received it.  The v2 revision of the linux-cxl patch (drop the Fixes:
> tag, struct_offset(), fail on a response too short for the header) does not
> apply to this one: this code is not in mainline, so there is no commit to point
> a Fixes: at, and the bound here already guards the subtraction against wrap.
> 
If the original send failed and did not make it to the mailing list, there's no
need to prefix this patch with RESEND.

Thanks,
Anisa
>  drivers/cxl/core/mbox.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 0b51a5d..c12ff86 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1764,6 +1764,7 @@ static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled)
>  	struct device *dev = mds->cxlds.dev;
>  	struct cxl_mbox_cmd mbox_cmd;
>  	u32 max_extent_count;
> +	size_t extents_hdr, max_returned;
>  	int latched_rc = 0;
>  	bool first = true;
>  
> @@ -1808,7 +1809,24 @@ static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled)
>  			first = false;
>  		}
>  
> +		/*
> +		 * The returned count is device-supplied: never index
> +		 * extent[] past the payload the device actually returned.
> +		 * The device also chooses the reported length and may return a
> +		 * response shorter than the header (min_out is 1), so derive the
> +		 * bound without underflowing.
> +		 */
> +		extents_hdr = offsetof(struct cxl_mbox_get_extent_out, extent);
> +		max_returned = mbox_cmd.size_out > extents_hdr ?
> +			       (mbox_cmd.size_out - extents_hdr) /
> +			       sizeof(struct cxl_extent) : 0;
>  		nr_returned = le32_to_cpu(extents->returned_extent_count);
> +		if (nr_returned > max_returned) {
> +			dev_warn_ratelimited(dev,
> +					     "Extent list: device claimed %u extents but the payload holds %zu\n",
> +					     nr_returned, max_returned);
> +			nr_returned = max_returned;
> +		}
>  		total_read += nr_returned;
>  		current_total = le32_to_cpu(extents->total_extent_count);
>  		current_gen_num = le32_to_cpu(extents->generation_num);
> @@ -1823,6 +1841,18 @@ static int __cxl_process_extent_list(struct cxl_endpoint_decoder *cxled)
>  			return -EAGAIN;
>  		}
>  
> +		/*
> +		 * A device that keeps claiming more extents without ever
> +		 * delivering any would otherwise spin this loop forever
> +		 * (the outer retry cannot help: this call never returns).
> +		 */
> +		if (!nr_returned && total_expected > total_read) {
> +			dev_warn_ratelimited(dev,
> +					     "Extent list: no progress after %u/%u extents; aborting\n",
> +					     total_read, total_expected);
> +			return -EIO;
> +		}
> +
>  		for (int i = 0; i < nr_returned ; i++) {
>  			struct cxl_extent *extent = &extents->extent[i];
>  
> -- 
> 2.34.1
>