[PATCH v3] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[PATCH v3] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Alison Schofield]

cxl_detach_ep() is called during bottom-up removal when all CXL memory
devices beneath a switch port have been removed. For each port in the
hierarchy it locks both the port and its parent, removes the endpoint,
and if the port is now empty, marks it dead and unregisters the port
by calling delete_switch_port(). There are two places during this work
where the parent_port may be used after freeing:

First, a concurrent detach may have already processed a port by the
time a second worker finds it via bus_find_device(). Without pinning
parent_port, it may already be freed when we discover port->dead and
attempt to unlock the parent_port. In a production kernel that's a
silent memory corruption, with lock debug, it looks like this:

[]DEBUG_LOCKS_WARN_ON(__owner_task(owner) != get_current())
[]WARNING: kernel/locking/mutex.c:949 at __mutex_unlock_slowpath+0x1ee/0x310
[]Call Trace:
[]mutex_unlock+0xd/0x20
[]cxl_detach_ep+0x180/0x400 [cxl_core]
[]devm_action_release+0x10/0x20
[]devres_release_all+0xa8/0xe0
[]device_unbind_cleanup+0xd/0xa0
[]really_probe+0x1a6/0x3e0

Second, delete_switch_port() releases three devm actions registered
against parent_port. The last of those is unregister_port() and it
calls device_unregister() on the child port, which can cascade. If
parent_port is now also empty the device core may unregister and free
it too. So by the time delete_switch_port() returns, parent_port may
be free, and the subsequent device_unlock(&parent_port->dev) operates
on freed memory. The kernel log looks same as above, with a different
offset in cxl_detach_ep().

Both of these issues stem from the absence of a lifetime guarantee
between a child port and its parent port.

Establish a lifetime rule for ports: child ports hold a reference to
their parent device until release. Take the reference when the port
is allocated and drop it when released. This ensures the parent is
valid for the full lifetime of the child and eliminates the use after
free window in cxl_detach_ep().

This is easily reproduced with a reload of cxl_acpi in QEMU with CXL
devices present.

Fixes: 2345df54249c ("cxl/memdev: Fix endpoint port removal")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Changes in v3:
Scope fix as a lifetime guarantee btw child port & parent (Dan)

Changes in v2:
Remove stray blank line (DaveJ)
Rebase on 7.0-rc1
Add Fixes Tag (DaveJ)


 drivers/cxl/core/port.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index b69c2529744c..09fc08205ed5 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -552,6 +552,10 @@ static void cxl_port_release(struct device *dev)
 	xa_destroy(&port->dports);
 	xa_destroy(&port->regions);
 	ida_free(&cxl_port_ida, port->id);
+
+	if (!is_cxl_root(port))
+		put_device(dev->parent);
+
 	if (is_cxl_root(port))
 		kfree(to_cxl_root(port));
 	else
@@ -721,6 +725,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev,
 		struct cxl_port *iter;
 
 		dev->parent = &parent_port->dev;
+		get_device(dev->parent);
 		port->depth = parent_port->depth + 1;
 		port->parent_dport = parent_dport;
 

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
-- 
2.37.3

Re: [PATCH v3] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Dave Jiang]

On 2/25/26 3:02 PM, Alison Schofield wrote:
> cxl_detach_ep() is called during bottom-up removal when all CXL memory
> devices beneath a switch port have been removed. For each port in the
> hierarchy it locks both the port and its parent, removes the endpoint,
> and if the port is now empty, marks it dead and unregisters the port
> by calling delete_switch_port(). There are two places during this work
> where the parent_port may be used after freeing:
> 
> First, a concurrent detach may have already processed a port by the
> time a second worker finds it via bus_find_device(). Without pinning
> parent_port, it may already be freed when we discover port->dead and
> attempt to unlock the parent_port. In a production kernel that's a
> silent memory corruption, with lock debug, it looks like this:
> 
> []DEBUG_LOCKS_WARN_ON(__owner_task(owner) != get_current())
> []WARNING: kernel/locking/mutex.c:949 at __mutex_unlock_slowpath+0x1ee/0x310
> []Call Trace:
> []mutex_unlock+0xd/0x20
> []cxl_detach_ep+0x180/0x400 [cxl_core]
> []devm_action_release+0x10/0x20
> []devres_release_all+0xa8/0xe0
> []device_unbind_cleanup+0xd/0xa0
> []really_probe+0x1a6/0x3e0
> 
> Second, delete_switch_port() releases three devm actions registered
> against parent_port. The last of those is unregister_port() and it
> calls device_unregister() on the child port, which can cascade. If
> parent_port is now also empty the device core may unregister and free
> it too. So by the time delete_switch_port() returns, parent_port may
> be free, and the subsequent device_unlock(&parent_port->dev) operates
> on freed memory. The kernel log looks same as above, with a different
> offset in cxl_detach_ep().
> 
> Both of these issues stem from the absence of a lifetime guarantee
> between a child port and its parent port.
> 
> Establish a lifetime rule for ports: child ports hold a reference to
> their parent device until release. Take the reference when the port
> is allocated and drop it when released. This ensures the parent is
> valid for the full lifetime of the child and eliminates the use after
> free window in cxl_detach_ep().
> 
> This is easily reproduced with a reload of cxl_acpi in QEMU with CXL
> devices present.
> 
> Fixes: 2345df54249c ("cxl/memdev: Fix endpoint port removal")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

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

> ---
> 
> Changes in v3:
> Scope fix as a lifetime guarantee btw child port & parent (Dan)
> 
> Changes in v2:
> Remove stray blank line (DaveJ)
> Rebase on 7.0-rc1
> Add Fixes Tag (DaveJ)
> 
> 
>  drivers/cxl/core/port.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index b69c2529744c..09fc08205ed5 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -552,6 +552,10 @@ static void cxl_port_release(struct device *dev)
>  	xa_destroy(&port->dports);
>  	xa_destroy(&port->regions);
>  	ida_free(&cxl_port_ida, port->id);
> +
> +	if (!is_cxl_root(port))
> +		put_device(dev->parent);
> +
>  	if (is_cxl_root(port))
>  		kfree(to_cxl_root(port));
>  	else
> @@ -721,6 +725,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev,
>  		struct cxl_port *iter;
>  
>  		dev->parent = &parent_port->dev;
> +		get_device(dev->parent);
>  		port->depth = parent_port->depth + 1;
>  		port->parent_dport = parent_dport;
>  
> 
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f

Re: [PATCH v3] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Li Ming]

On 2/26/2026 6:02 AM, Alison Schofield wrote:
> cxl_detach_ep() is called during bottom-up removal when all CXL memory
> devices beneath a switch port have been removed. For each port in the
> hierarchy it locks both the port and its parent, removes the endpoint,
> and if the port is now empty, marks it dead and unregisters the port
> by calling delete_switch_port(). There are two places during this work
> where the parent_port may be used after freeing:
>
> First, a concurrent detach may have already processed a port by the
> time a second worker finds it via bus_find_device(). Without pinning
> parent_port, it may already be freed when we discover port->dead and
> attempt to unlock the parent_port. In a production kernel that's a
> silent memory corruption, with lock debug, it looks like this:
>
> []DEBUG_LOCKS_WARN_ON(__owner_task(owner) != get_current())
> []WARNING: kernel/locking/mutex.c:949 at __mutex_unlock_slowpath+0x1ee/0x310
> []Call Trace:
> []mutex_unlock+0xd/0x20
> []cxl_detach_ep+0x180/0x400 [cxl_core]
> []devm_action_release+0x10/0x20
> []devres_release_all+0xa8/0xe0
> []device_unbind_cleanup+0xd/0xa0
> []really_probe+0x1a6/0x3e0
>
> Second, delete_switch_port() releases three devm actions registered
> against parent_port. The last of those is unregister_port() and it
> calls device_unregister() on the child port, which can cascade. If
> parent_port is now also empty the device core may unregister and free
> it too. So by the time delete_switch_port() returns, parent_port may
> be free, and the subsequent device_unlock(&parent_port->dev) operates
> on freed memory. The kernel log looks same as above, with a different
> offset in cxl_detach_ep().
>
> Both of these issues stem from the absence of a lifetime guarantee
> between a child port and its parent port.
>
> Establish a lifetime rule for ports: child ports hold a reference to
> their parent device until release. Take the reference when the port
> is allocated and drop it when released. This ensures the parent is
> valid for the full lifetime of the child and eliminates the use after
> free window in cxl_detach_ep().
>
> This is easily reproduced with a reload of cxl_acpi in QEMU with CXL
> devices present.
>
> Fixes: 2345df54249c ("cxl/memdev: Fix endpoint port removal")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Li Ming <ming.li@zohomail.com>

A minor comment below

> ---
>
> Changes in v3:
> Scope fix as a lifetime guarantee btw child port & parent (Dan)
>
> Changes in v2:
> Remove stray blank line (DaveJ)
> Rebase on 7.0-rc1
> Add Fixes Tag (DaveJ)
>
>
>  drivers/cxl/core/port.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index b69c2529744c..09fc08205ed5 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -552,6 +552,10 @@ static void cxl_port_release(struct device *dev)
>  	xa_destroy(&port->dports);
>  	xa_destroy(&port->regions);
>  	ida_free(&cxl_port_ida, port->id);
> +
> +	if (!is_cxl_root(port))
> +		put_device(dev->parent);
> +
>  	if (is_cxl_root(port))
>  		kfree(to_cxl_root(port));
>  	else

There is a if-else block to handle cxl_root case and non-cxl_root case, maybe we can move this put_device() into this else block.


Ming

[snip]

Re: [PATCH v3] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Alison Schofield]

On Thu, Feb 26, 2026 at 05:37:42PM +0800, Li Ming wrote:

snip

> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index b69c2529744c..09fc08205ed5 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -552,6 +552,10 @@ static void cxl_port_release(struct device *dev)
> >  	xa_destroy(&port->dports);
> >  	xa_destroy(&port->regions);
> >  	ida_free(&cxl_port_ida, port->id);
> > +
> > +	if (!is_cxl_root(port))
> > +		put_device(dev->parent);
> > +
> >  	if (is_cxl_root(port))
> >  		kfree(to_cxl_root(port));
> >  	else
> 
> There is a if-else block to handle cxl_root case and non-cxl_root case, maybe we can move this put_device() into this else block.
> 
Thanks Ming!  Fixed in v4.


> 
> Ming
> 
> [snip]
>