Re: [PATCH 1/9] cxl/region: Fix use-after-free from auto assembly failure

[Dave Jiang <dave.jiang@intel.com>]

On 3/26/26 10:28 PM, Dan Williams wrote:
> The following crash signature results from region destruction while an
> endpoint decoder is staged, but not fully attached.
> 
> ---
>  BUG: KASAN: slab-use-after-free in __cxl_decoder_detach+0x724/0x830 [cxl_core]
>  Read of size 8 at addr ffff888265638840 by task modprobe/1287
> 
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x68/0x90
>   print_report+0x170/0x4e2
>   kasan_report+0xc2/0x1a0
>   __cxl_decoder_detach+0x724/0x830 [cxl_core]
>   cxl_decoder_detach+0x6c/0x100 [cxl_core]
>   unregister_region+0x88/0x140 [cxl_core]
>   devres_release_all+0x172/0x230
> ---
> 
> The "staged" state is established by cxl_region_attach_auto() and finalized
> by cxl_region_attach_position(). When that is finalized a memdev removal
> event will destroy regions before endpoint decoders. However, in the
> interim the memdev removal will falsely assume that the endpoint decoder is
> unattached. Later, the eventual region removal finds the stale pointer to
> the now freed endpoint decoder.
> 
> Introduce CXL_DECODER_STATE_AUTO_STAGED and cxl_cancel_auto_attach() to
> cleanup this interim state.
> 
> Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
> Cc: <stable@vger.kernel.org>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>  drivers/cxl/cxl.h         |  6 +++--
>  drivers/cxl/core/region.c | 54 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9b947286eb9b..30a31968f266 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -378,12 +378,14 @@ struct cxl_decoder {
>  };
>  
>  /*
> - * Track whether this decoder is reserved for region autodiscovery, or
> - * free for userspace provisioning.
> + * Track whether this decoder is free for userspace provisioning, reserved for
> + * region autodiscovery, whether it is started connecting (awaiting other
> + * peers), or has completed auto assembly.
>   */
>  enum cxl_decoder_state {
>  	CXL_DECODER_STATE_MANUAL,
>  	CXL_DECODER_STATE_AUTO,
> +	CXL_DECODER_STATE_AUTO_STAGED,
>  };
>  
>  /**
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f7b20f60ac5c..b72556c1458b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1064,6 +1064,14 @@ static int cxl_rr_ep_add(struct cxl_region_ref *cxl_rr,
>  
>  	if (!cxld->region) {
>  		cxld->region = cxlr;
> +
> +		/*
> +		 * Now that cxld->region is set the intermediate staging state
> +		 * can be cleared.
> +		 */
> +		if (cxld == &cxled->cxld &&
> +		    cxled->state == CXL_DECODER_STATE_AUTO_STAGED)
> +			cxled->state = CXL_DECODER_STATE_AUTO;
>  		get_device(&cxlr->dev);
>  	}
>  
> @@ -1805,6 +1813,7 @@ static int cxl_region_attach_auto(struct cxl_region *cxlr,
>  	pos = p->nr_targets;
>  	p->targets[pos] = cxled;
>  	cxled->pos = pos;
> +	cxled->state = CXL_DECODER_STATE_AUTO_STAGED;
>  	p->nr_targets++;
>  
>  	return 0;
> @@ -2154,6 +2163,47 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	return 0;
>  }
>  
> +static int cxl_region_by_target(struct device *dev, const void *data)
> +{
> +	const struct cxl_endpoint_decoder *cxled = data;
> +	struct cxl_region_params *p;
> +	struct cxl_region *cxlr;
> +
> +	if (!is_cxl_region(dev))
> +		return 0;
> +
> +	cxlr = to_cxl_region(dev);
> +	p = &cxlr->params;
> +	return p->targets[cxled->pos] == cxled;
> +}
> +
> +/*
> + * When an auto-region fails to assemble the decoder may be listed as a target,
> + * but not fully attached.
> + */
> +static void cxl_cancel_auto_attach(struct cxl_endpoint_decoder *cxled)
> +{
> +	struct cxl_region_params *p;
> +	struct cxl_region *cxlr;
> +	int pos = cxled->pos;
> +
> +	if (cxled->state != CXL_DECODER_STATE_AUTO_STAGED)
> +		return;
> +
> +	struct device *dev __free(put_device) = bus_find_device(
> +		&cxl_bus_type, NULL, cxled, cxl_region_by_target);
> +	if (!dev)
> +		return;
> +
> +	cxlr = to_cxl_region(dev);
> +	p = &cxlr->params;
> +
> +	p->nr_targets--;
> +	cxled->state = CXL_DECODER_STATE_AUTO;
> +	cxled->pos = -1;
> +	p->targets[pos] = NULL;
> +}
> +
>  static struct cxl_region *
>  __cxl_decoder_detach(struct cxl_region *cxlr,
>  		     struct cxl_endpoint_decoder *cxled, int pos,
> @@ -2177,8 +2227,10 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
>  		cxled = p->targets[pos];
>  	} else {
>  		cxlr = cxled->cxld.region;
> -		if (!cxlr)
> +		if (!cxlr) {
> +			cxl_cancel_auto_attach(cxled);
>  			return NULL;
> +		}
>  		p = &cxlr->params;
>  	}
>