Re: [RFC PATCH 1/3] cxl/region: Reset software-created regions on memdev detach

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Userspace can bypass the F_LOCK protection intended to prevent userspace disposition, resulting in permanently leaked hardware decoders and potential memory corruption.
--

--- Patch [1]: [RFC PATCH 1/3] cxl/region: Reset software-created regions on memdev detach ---
commit f2541005d7a5430e1aac6ecade8931768b282855
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Reset software-created regions on memdev detach

This patch modifies region teardown to pass a reset context (DEFAULT vs
MANAGED_DETACH). It allows a memdev attach provider to reset its own
software-created regions without changing generic teardown behavior.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..7fcaddc61180d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2893,7 +2963,7 @@ static ssize_t delete_region_store(struct device *dev,
>  	if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev)))
>  		return -ENODEV;
>  
> -	unregister_region(cxlr);
> +	unregister_region(cxlr, CXL_REGION_RESET_DEFAULT);
>  
>  	return len;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this sysfs path allow
userspace to bypass the F_LOCK protection and corrupt memory?

When a user writes to the delete_region sysfs file, delete_region_store()
unconditionally calls unregister_region() with CXL_REGION_RESET_DEFAULT,
even if CXL_REGION_F_LOCK is set.

This propagates down to cxl_region_decode_reset(), where the combination
of F_LOCK and CXL_REGION_RESET_DEFAULT causes an early return, skipping
the hardware reset:

drivers/cxl/core/region.c:cxl_region_decode_reset() {
        if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags) &&
            (context != CXL_REGION_RESET_MANAGED_DETACH || ...))
                return;
        ...
}

However, the software teardown still completes in __cxl_decoder_detach()
and the region is unregistered, freeing the HPA range for reallocation:

drivers/cxl/core/region.c:__cxl_decoder_detach() {
        ...
        if (p->state == CXL_CONFIG_ACTIVE) {
                p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
                cxl_region_teardown_targets(cxlr);
        }
        p->targets[cxled->pos] = NULL;
        p->nr_targets--;
        ...
}

Could this leave the hardware decoders permanently programmed while the
software structures are destroyed? If the kernel reallocates this HPA range
to a new region, would it create overlapping hardware decoders and lead
to memory corruption?

The same bypass pattern appears to be preserved in store_targetN() when
detaching targets. Should these sysfs paths verify CXL_REGION_F_LOCK
before initiating teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805074042.30173-1-icheng@nvidia.com?part=1