Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The dax_resource devm cleanup action is tied to the dc_extent device instead of the DAX region, leading to a Use-After-Free of the DAX region if the DAX driver unbinds. --
commit 20b0e06e09f045b8ed2d61cce6004f2c3647ec5a Author: Ira Weiny <[email protected]> cxl + dax: Surface dax_resources on DCD Add Capacity events This commit defines the DAX region .notify callback and registers a dax_resource for each extent when the CXL side onlines a tag group during a DC Add event. > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index 7356aaaffe577..9b5c03616b83d 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c [ ... ] > @@ -182,6 +182,138 @@ static bool is_dynamic(struct dax_region *dax_region) > return (dax_region->res.flags & IORESOURCE_DAX_DCD) != 0; > } > > +static void __dax_release_resource(struct dax_resource *dax_resource) > +{ > + struct dax_region *dax_region = dax_resource->region; > + > + lockdep_assert_held_write(&dax_region_rwsem); > + dev_dbg(dax_region->dev, "Extent release resource %pr\n", > + dax_resource->res); [ ... ] > +static int __dax_region_add_resource(struct dax_region *dax_region, > + struct device *device, > + resource_size_t start, resource_size_t > length, > + const uuid_t *tag, u16 seq_num) > +{ > + struct dax_resource *dax_resource __free(kfree) = > + kzalloc(sizeof(*dax_resource), GFP_KERNEL); [ ... ] > + /* > + * open code devm_add_action_or_reset() to avoid recursive write lock > + * of dax_region_rwsem in the error case. > + */ > + rc = devm_add_action(device, dax_release_resource, dax_resource); [Severity: High] Does this tie the cleanup action to the wrong device lifetime? In __dax_region_add_resource(), the devm_add_action() registers dax_release_resource() to the passed-in device, which is the dc_extent device managed by the CXL core. However, the dax_region is tied to the DAX driver's lifecycle. If a user unbinds the DAX driver, the dax_region is freed. Because the dc_extent device is managed externally, it outlives the DAX driver unbind. Later, when the CXL region is destroyed and the dc_extent is released, dax_release_resource() will run and call __dax_release_resource(), which dereferences the freed dax_region: __dax_release_resource() struct dax_region *dax_region = dax_resource->region; dev_dbg(dax_region->dev, ...); Can this lead to a use-after-free on the dax_region? > + if (rc) { > + __dax_release_resource(dax_resource); > + return rc; > + } > + > + dev_set_drvdata(device, no_free_ptr(dax_resource)); > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=21
