Re: [PATCH v11 07/31] cxl/region: Add DC DAX region support

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