Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Lockless access to `cxlmd->cxlds` via the newly introduced `cxled_to_mds()` helper in `store_targetN()` leads to a NULL pointer dereference during concurrent device teardown. --
commit 0243c2c435424873e37aafdcb17aee31b0ea07a1 Author: Ira Weiny <[email protected]> cxl/region: Add DC DAX region support This commit introduces dynamic DAX regions to support CXL DC capabilities. It adds a new sysfs attribute for creating dynamic RAM regions and updates target attachment logic to handle DCD support. > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h > index 82ca3a4767080..9ed141fa13341 100644 > --- a/drivers/cxl/core/core.h > +++ b/drivers/cxl/core/core.h [ ... ] > @@ -18,6 +19,13 @@ enum cxl_detach_mode { > DETACH_INVALIDATE, > }; > > +static inline struct cxl_memdev_state * > +cxled_to_mds(struct cxl_endpoint_decoder *cxled) > +{ > + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > + return to_cxl_memdev_state(cxlmd->cxlds); > +} [Severity: High] Can this result in a NULL pointer dereference during concurrent device teardown? If a user writes to a CXL region's targetX sysfs file, store_targetN() calls cxled_to_mds() without holding locks that synchronize with device teardown. Concurrently, if the CXL device is hot-removed or the driver is unbound, cxl_memdev_shutdown() nullifies cxlmd->cxlds under cxl_memdev_rwsem. This would cause cxled_to_mds() to pass a NULL pointer to to_cxl_memdev_state(), which unconditionally dereferences it: drivers/cxl/cxlmem.h:to_cxl_memdev_state() { if (cxlds->type != CXL_DEVTYPE_CLASSMEM) return NULL; ... } Does this path need to hold cxl_memdev_rwsem or otherwise verify cxlmd->cxlds is not NULL before using it? > + > #ifdef CONFIG_CXL_REGION > > struct cxl_region_context { [ ... ] > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index e50dc716d4e82..ba03ec5e27c32 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c [ ... ] > @@ -2389,6 +2396,8 @@ static size_t store_targetN(struct cxl_region *cxlr, > const char *buf, int pos, > if (sysfs_streq(buf, "\n")) > rc = detach_target(cxlr, pos); > else { > + struct cxl_endpoint_decoder *cxled; > + struct cxl_memdev_state *mds; > struct device *dev; > > dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf); > @@ -2400,8 +2409,21 @@ static size_t store_targetN(struct cxl_region *cxlr, > const char *buf, int pos, > goto out; > } > > - rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos, > - TASK_INTERRUPTIBLE); > + cxled = to_cxl_endpoint_decoder(dev); > + if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM_1) { > + mds = cxled_to_mds(cxled); > + if (!mds) { > + dev_dbg(dev, "No memdev state\n"); > + rc = -ENODEV; > + goto out; > + } > + if (!cxl_dcd_supported(mds)) { > + dev_dbg(dev, "DCD unsupported\n"); > + rc = -EINVAL; > + goto out; > + } > + } > + rc = attach_target(cxlr, cxled, pos, TASK_INTERRUPTIBLE); > out: > put_device(dev); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
