[PATCH] cxl/region: Reject delete of a provider-locked region

[PATCH] cxl/region: Reject delete of a provider-locked region

[Richard Cheng]

delete_region_store() destroys a region object without checking
CXL_REGION_F_LOCK, unlike commit_store() which refuses locked regions
with -EPERM. A region owned by a memdev-attach provider carries F_LOCK,
and cxl_region_decode_reset() deliberately skips the HDM decoder reset
for F_LOCK regions. As a result, a userspace write to delete_region
tears down the region object but leaves the endpoint HDM decoder
committed with no owning region, which is an orphan, enabled decoder.

Gate delete_region_store() on CXL_REGION_F_LOCK.

Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 578622240401..cfc9622c9309 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2894,6 +2894,9 @@ static ssize_t delete_region_store(struct device *dev,
 	if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev)))
 		return -ENODEV;
 
+	if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags))
+		return -EPERM;
+
 	unregister_region(cxlr);
 
 	return len;

base-commit: 5ca04f3ba91f1773bbd5da6d9c654ccc1ba7831d
-- 
2.43.0

Re: [PATCH] cxl/region: Reject delete of a provider-locked region

[sashiko-bot]

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

New issues:
- [High] A Time-Of-Check to Time-Of-Use (TOCTOU) race condition in `delete_region_store()` allows userspace to bypass the `CXL_REGION_F_LOCK` protection, leading to orphaned hardware decoders.

Pre-existing issues:
- [High] Userspace can bypass `CXL_REGION_F_LOCK` restrictions and tear down region software state by manually detaching targets via `targetN` sysfs attributes, leading to orphaned hardware decoders.
--

--- Patch [1]: [PATCH] cxl/region: Reject delete of a provider-locked region ---
commit 447746e2d3844401b6bdd5604c498e319f1e7719
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Reject delete of a provider-locked region
    
delete_region_store() destroys a region object without checking
CXL_REGION_F_LOCK, unlike commit_store() which refuses locked regions
with -EPERM. This patch gates delete_region_store() on CXL_REGION_F_LOCK.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 578622240401d..cfc9622c93095 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2894,6 +2894,9 @@ static ssize_t delete_region_store(struct device *dev,
>  	if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev)))
>  		return -ENODEV;
>  
> +	if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags))
> +		return -EPERM;
> +

[Severity: High]
Is there a Time-Of-Check to Time-Of-Use (TOCTOU) race condition here?

The CXL_REGION_F_LOCK check is performed without holding synchronization
like the region rwsem, leaving a window open before unregister_region()
completes.

If userspace writes to delete_region while concurrently writing to targetN
to attach a target, delete_region_store() could check the lock flag before
it is set.

Then unregister_region() calling device_del() can block in kernfs_drain
until the concurrent target attachment finishes and sets CXL_REGION_F_LOCK.

Once resumed, the software region would be destroyed with the lock flag
set, skipping the hardware reset. Could this sequence recreate the exact
orphaned hardware decoder state this patch intended to prevent?


[Severity: High]
This is a pre-existing issue, but does a similar bypass exist in
store_targetN() when userspace manually detaches targets?

If a root user writes \n to a targetN sysfs attribute of a locked region,
store_targetN() calls detach_target() which eventually reaches
__cxl_decoder_detach(). This software teardown path lacks an F_LOCK check.

Because cxl_region_decode_reset() deliberately skips the hardware reset
for F_LOCK regions, the software state is dropped while the hardware
decoders remain committed.

The user could then write 0 to the size attribute, freeing the physical
memory while it is still actively decoded by hardware.

Should store_targetN() or __cxl_decoder_detach() also be gated by
CXL_REGION_F_LOCK to prevent userspace from tearing down the software
state of locked regions?

>  	unregister_region(cxlr);
>  
>  	return len;

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

Re: [PATCH] cxl/region: Reject delete of a provider-locked region

[Richard Cheng]

On Tue, Jul 21, 2026 at 06:04:02AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] A Time-Of-Check to Time-Of-Use (TOCTOU) race condition in `delete_region_store()` allows userspace to bypass the `CXL_REGION_F_LOCK` protection, leading to orphaned hardware decoders.
> 
> Pre-existing issues:
> - [High] Userspace can bypass `CXL_REGION_F_LOCK` restrictions and tear down region software state by manually detaching targets via `targetN` sysfs attributes, leading to orphaned hardware decoders.
> --
> 
> --- Patch [1]: [PATCH] cxl/region: Reject delete of a provider-locked region ---
> commit 447746e2d3844401b6bdd5604c498e319f1e7719
> Author: Richard Cheng <icheng@nvidia.com>
> 
> cxl/region: Reject delete of a provider-locked region
>     
> delete_region_store() destroys a region object without checking
> CXL_REGION_F_LOCK, unlike commit_store() which refuses locked regions
> with -EPERM. This patch gates delete_region_store() on CXL_REGION_F_LOCK.
> 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 578622240401d..cfc9622c93095 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -2894,6 +2894,9 @@ static ssize_t delete_region_store(struct device *dev,
> >  	if (!cxlr || !sysfs_streq(buf, dev_name(&cxlr->dev)))
> >  		return -ENODEV;
> >  
> > +	if (test_bit(CXL_REGION_F_LOCK, &cxlr->flags))
> > +		return -EPERM;
> > +
> 
> [Severity: High]
> Is there a Time-Of-Check to Time-Of-Use (TOCTOU) race condition here?
> 
> The CXL_REGION_F_LOCK check is performed without holding synchronization
> like the region rwsem, leaving a window open before unregister_region()
> completes.
> 
> If userspace writes to delete_region while concurrently writing to targetN
> to attach a target, delete_region_store() could check the lock flag before
> it is set.
> 
> Then unregister_region() calling device_del() can block in kernfs_drain
> until the concurrent target attachment finishes and sets CXL_REGION_F_LOCK.
> 
> Once resumed, the software region would be destroyed with the lock flag
> set, skipping the hardware reset. Could this sequence recreate the exact
> orphaned hardware decoder state this patch intended to prevent?
>

I'd push back on this.

I think check-then-act observation is fair, but it doesn't affect the patch's
target, which is a provider-locked region that is F_LOCK'd during assembly
before any userspace delete is possible, and F_LOCK is never cleared,
so the flag is always already set when delete_region_store() runs for it.

The window described by the sashiko-bot requires userspace to concurrently
attach an F_LOCK-setting target to the same region.
I don't see a reachable path for that. And it can't be closed with a lock
in delete_region_store() because unregister_region() calls device_del()
before reset.
 
> 
> [Severity: High]
> This is a pre-existing issue, but does a similar bypass exist in
> store_targetN() when userspace manually detaches targets?
> 
> If a root user writes \n to a targetN sysfs attribute of a locked region,
> store_targetN() calls detach_target() which eventually reaches
> __cxl_decoder_detach(). This software teardown path lacks an F_LOCK check.
> 
> Because cxl_region_decode_reset() deliberately skips the hardware reset
> for F_LOCK regions, the software state is dropped while the hardware
> decoders remain committed.
> 
> The user could then write 0 to the size attribute, freeing the physical
> memory while it is still actively decoded by hardware.
> 
> Should store_targetN() or __cxl_decoder_detach() also be gated by
> CXL_REGION_F_LOCK to prevent userspace from tearing down the software
> state of locked regions?
> 
> >  	unregister_region(cxlr);
> >  
> >  	return len;
> 

Will send another patch fix for this one.

Best regards,
Richard Cheng.

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