Re: [PATCH v1 10/14] cxl/extent: Enforce cross-region tag uniqueness

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl/extent: same-chain tag group can span multiple regions in cxlr_add_extent()
--

commit dccf3410f0df24895d72657ec87c46291cc4a6bc
Author: Anisa Su <anisa.su887@gmail.com>

cxl/extent: Enforce cross-region tag uniqueness

This commit adds a host-wide registry of live non-null tag groups protected
by cxl_tag_lock to enforce tag UUID uniqueness across regions and memdevs.
During group validation, cxl_tag_already_committed() rejects groups whose tag
UUID is already registered with -EEXIST.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index a1d3e04a1e1b4..0fa501f5800c9 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1640,6 +1640,22 @@ static int cxl_validate_group(struct cxl_memdev_state *mds, const uuid_t *tag,
>  	struct device *dev = mds->cxlds.dev;
>  	struct cxl_extent_list_node *pos;
>  
> +	/*
> +	 * Cross-More-chain uniqueness.  A non-null tag seen in this group must
> +	 * not already correspond to a committed tag group anywhere on this
> +	 * host.  More=0 was supposed to close that allocation, and tag uuids
> +	 * must be unique across all regions and memdevs (the orchestrator owns
> +	 * assignment per spec).  Either constraint failing — same chain
> +	 * redelivered, or two distinct allocations colliding on the same uuid —
> +	 * is a firmware/orchestrator bug; reject the whole group.
> +	 */
> +	if (cxl_tag_already_committed(tag)) {
> +		dev_warn_ratelimited(dev,
> +				     "Tag %pUb: dropping group, tag already committed (firmware/orchestrator bug)\n",
> +				     tag);
> +		return -EEXIST;
> +	}

[Severity: High]
Can a single More-chain still create a tag group that spans multiple regions?

A single DC partition on a memdev can be decoded by multiple endpoint
decoders attached to different regions (for example cxlr_dax0 and cxlr_dax1)
by selecting the same DC partition mode via cxl_dpa_set_part() and allocating
disjoint DPA ranges from the partition resource via cxl_dpa_alloc().

If a DCD Add Capacity More-chain arrives with two extents sharing the same
non-null tag UUID whose DPAs fall into cxled0 (in cxlr_dax0) and cxled1 (in
cxlr_dax1) on that same DC partition:

1. In cxl_add_pending(), extract_tag_group() groups both extents together.

2. In cxl_validate_group(), cxl_tag_already_committed(tag) returns false
because the group has not been allocated or registered in cxl_tag_groups yet,
and cxl_check_group_partition() returns 0 because both extents resolve to the
same struct cxl_dpa_partition.

3. In cxl_realize_group() -> cxl_add_extent() -> cxlr_add_extent(), the first
extent allocates the tag group on cxlr_dax0 and registers the UUID, while the
second extent resolves to cxlr_dax1 and dc_extent_build() computes
dc_extent->hpa_range relative to cxlr_dax1->hpa_range.start.

4. When cxlr_add_extent() is called for the second extent, it only checks
uuid_equal() and does not check whether (*group)->cxlr_dax matches cxlr_dax:

drivers/cxl/core/extent.c:cxlr_add_extent() {
	if (*group && !uuid_equal(&(*group)->uuid, &dc_extent->uuid)) {
		kfree(dc_extent);
		return -EINVAL;
	}

	if (!*group) {
		dev_dbg(&cxlr_dax->dev, "Alloc new tag group\n");
		*group = alloc_tag_group(cxlr_dax, &dc_extent->uuid);
		...
	} else {
		dev_dbg(&cxlr_dax->dev, "Append dc_extent to tag group\n");
	}
}

As a result, online_tag_group() inserts the second extent into
cxlr_dax0->dc_extents with an HPA offset computed from cxlr_dax1, and only
pins and registers dc_tag_group_teardown on cxlr_dax0->dev. If cxlr_dax1 and
cxled1 are later torn down while cxlr_dax0 remains active, the second
dc_extent is not torn down and retains a dangling dc_extent->cxled pointer.

Should cxlr_add_extent() (or cxl_validate_group()) also verify that all
extents in the group resolve to the same cxlr_dax region?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104230445.66625-1-anisa.su@samsung.com?part=10