On Thu, Jun 25, 2026 at 06:16:49PM +0000, [email protected] wrote: > 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.
Fixed. > -- > > 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? > Fixed: Added new helper function cxled_dcd_check() in memdev.c instead of called cxled_to_mds(). The new helper grabs the lock before reading cxlmd->cxlds: int cxled_dcd_check(struct cxl_endpoint_decoder *cxled) { struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); struct cxl_dev_state *cxlds; guard(rwsem_read)(&cxl_memdev_rwsem); cxlds = cxlmd->cxlds; if (!cxlds || cxlds->type != CXL_DEVTYPE_CLASSMEM) return -ENODEV; if (!cxl_dcd_supported(to_cxl_memdev_state(cxlds))) return -EINVAL; return 0; } EXPORT_SYMBOL_NS_GPL(cxled_dcd_check, "CXL"); Exported to cxl/core/region.c so it can be called from store_targetN() for dcd checks: if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM_1) { rc = cxled_dcd_check(cxled); if (rc == -ENODEV) dev_dbg(dev, "No memdev state\n"); else if (rc == -EINVAL) dev_dbg(dev, "DCD unsupported\n"); if (rc) goto out; } > > + > > #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
