Re: [PATCH 1/2] cxl/region: Reject a repeat write of an endpoint decoder to targetN

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 30 Sep 2026 21:56:38 -0700
Alison Schofield <alison.schofield@intel.com> wrote:

> Writing an endpoint decoder to a region targetN attribute that already
> holds it returns -EBUSY, but leaves the partially populated region in an
> inconsistent state. The targetN attribute still names the decoder, so the
> failed write gives no indication that the region was damaged.
> 
> Filling the remaining target positions can then trigger a NULL pointer
> dereference in cxl_region_setup_targets(). Root can reproduce this
> through sysfs:
> 
>   echo $decoder > /sys/bus/cxl/devices/region0/target0
>   echo $decoder > /sys/bus/cxl/devices/region0/target0
>   echo $other_decoder > /sys/bus/cxl/devices/region0/target1
> 
> Reject a write to an already assigned position before modifying the region.

Rejecting a write that does nothing with an error isn't the nicest of
ABI choices - not to mention it's an ABI change (even if doesn't matter
much here given previously it probably crashed :).

Can we instead just reject much earlier before doing anything that
needs tearing down?  Would moving the check to the top of
cxl_region_attach() work?  There are some queries before that but
I don't think any of them update state and I think we
already have the stuff needed for a sanity check and early bail
out at that point.

I'm too lazy to actually test that theory so may well have missed something!

Thanks,

Jonathan



> 
> Fixes: b9686e8c8e39 ("cxl/region: Enable the assignment of endpoint decoders to regions")
> Assisted-by: LLM
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> 
> An ndctl patch backstopping this fix in the cxl unit tests will follow.
> 
> 
> 
>  drivers/cxl/core/region.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c..f9aafe89e5bb 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1762,9 +1762,6 @@ static int cxl_region_validate_position(struct cxl_region *cxlr,
>  		return -ENXIO;
>  	}
>  
> -	if (p->targets[pos] == cxled)
> -		return 0;
> -
>  	if (p->targets[pos]) {
>  		struct cxl_endpoint_decoder *cxled_target = p->targets[pos];
>  		struct cxl_memdev *cxlmd_target = cxled_to_memdev(cxled_target);
> 
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935