Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Use-after-free of `tag_group` in `cxl_realize_group()` when extent addition fails. - [High] Data race on `group->nr_extents` during concurrent extent device release leading to UAF/leak. --
commit 0105c4aa2861118ac775e4aaac54f4e888f84aa2 Author: Anisa Su <[email protected]> cxl/extent: Enforce cross-region tag uniqueness This patch adds a host-wide registry of live tag groups with non-null uuids to ensure that tag uuids are unique across all regions and memdevs. It updates alloc_tag_group() to register new tags, free_tag_group() to unregister them, and validation logic to reject conflicting tags. > diff --git a/drivers/cxl/core/extent.c b/drivers/cxl/core/extent.c > index a590a89f35802..36be56ca10974 100644 > --- a/drivers/cxl/core/extent.c > +++ b/drivers/cxl/core/extent.c [ ... ] > static void free_tag_group(struct cxl_dc_tag_group *group) > { > + cxl_tag_unregister(group); > xa_destroy(&group->dc_extents); > /* Drop the pin taken in alloc_tag_group(). */ > put_device(&group->cxlr_dax->dev); [Severity: High] Can this lead to a use-after-free of the tag_group pointer? If xa_insert() fails (e.g., due to -ENOMEM) for the first extent in cxlr_add_extent(), the allocated tag_group is retained in mds->add_ctx.group but its dc_extents xarray remains empty. Back in cxl_realize_group(), the loop continues and eventually extracts this empty tag_group: cxl_realize_group() { ... tag_group = mds->add_ctx.group; ... rc = online_tag_group(tag_group, !existing); ... if (existing) rc = __cxlr_notify_extent(tag_group->cxlr_dax->cxlr, ... } If online_tag_group() iterates over the empty xarray, decrements group->nr_extents to 0, frees the tag_group via free_tag_group(), and returns 0 (success), won't cxl_realize_group() then proceed to dereference the freed tag_group pointer when calling __cxlr_notify_extent() or cxlr_notify_extent()? [Severity: High] Can a data race occur on group->nr_extents during concurrent releases? The group->nr_extents counter is decremented with a non-atomic group->nr_extents-- in both dc_extent_release() and rm_tag_group(). Because sysfs files like length and offset are world-readable, unprivileged userspace processes can hold references to multiple extents within the same tag group. If these processes close the sysfs files concurrently, dc_extent_release() executes on multiple CPUs concurrently. If one CPU reads the counter, and a second CPU reads, decrements, and frees the tag_group via free_tag_group(), won't the first CPU then write to the nr_extents field of the freed memory? > kfree(group); > } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
