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

[PATCH v4] 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")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Changes in v4:
Move the put_device() into existing if-else logic (Ming)

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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index b69c2529744c..a004da8fbe55 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -552,10 +552,13 @@ 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))
+
+	if (is_cxl_root(port)) {
 		kfree(to_cxl_root(port));
-	else
+	} else {
+		put_device(dev->parent);
 		kfree(port);
+	}
 }
 
 static ssize_t decoders_committed_show(struct device *dev,
@@ -721,6 +724,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 v4] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Jonathan Cameron]

On Thu, 26 Feb 2026 10:44:36 -0800
Alison Schofield <alison.schofield@intel.com> 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")
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
New rule makes sense to me and implementation looks good.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

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

[Li Ming]

On 2/27/2026 2:44 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().

Hi Alison,

After looking deeper into these cases, looks like the changes cannot cover the second case?

My understanding is that cxl_port_release() will be called on the child port before the device_unlock(&parent_port->dev). The put_device() for the parent_port is still possible to release parent_port before calling device_unlock(&parent_port->dev).

If my understanding is wrong, please correct me. Thanks.


Ming

> 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")
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>
> Changes in v4:
> Move the put_device() into existing if-else logic (Ming)
>
> 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 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index b69c2529744c..a004da8fbe55 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -552,10 +552,13 @@ 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))
> +
> +	if (is_cxl_root(port)) {
>  		kfree(to_cxl_root(port));
> -	else
> +	} else {
> +		put_device(dev->parent);
>  		kfree(port);
> +	}
>  }
>  
>  static ssize_t decoders_committed_show(struct device *dev,
> @@ -721,6 +724,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 v4] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Alison Schofield]

On Sat, Feb 28, 2026 at 03:04:14PM +0800, Li Ming wrote:
> On 2/27/2026 2:44 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().
> 
> Hi Alison,
> 
> After looking deeper into these cases, looks like the changes cannot cover the second case?
> 
> My understanding is that cxl_port_release() will be called on the child port before the device_unlock(&parent_port->dev). The put_device() for the parent_port is still possible to release parent_port before calling device_unlock(&parent_port->dev).

Thanks for digging in Ming! I see what you mean. It is possible for the
parent_port to be released before the device_unlock of parent port. In
v5 I've brought back a tiny piece of the v1 by taking an explicit ref
on parent port across the device lock/unlock in cxl_detach_ep(). That'll
make the second case safe independent of unregister ordering/cascading.

-- Alison


> 
> If my understanding is wrong, please correct me. Thanks.
> 
> 
> Ming
> 
> > 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")
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Li Ming <ming.li@zohomail.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >
> > Changes in v4:
> > Move the put_device() into existing if-else logic (Ming)
> >
> > 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 | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index b69c2529744c..a004da8fbe55 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -552,10 +552,13 @@ 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))
> > +
> > +	if (is_cxl_root(port)) {
> >  		kfree(to_cxl_root(port));
> > -	else
> > +	} else {
> > +		put_device(dev->parent);
> >  		kfree(port);
> > +	}
> >  }
> >  
> >  static ssize_t decoders_committed_show(struct device *dev,
> > @@ -721,6 +724,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 v4] cxl/port: Fix use after free of parent_port in cxl_detach_ep()

[Dave Jiang]

On 2/26/26 11:44 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")
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> Changes in v4:
> Move the put_device() into existing if-else logic (Ming)
> 
> 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 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index b69c2529744c..a004da8fbe55 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -552,10 +552,13 @@ 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))
> +
> +	if (is_cxl_root(port)) {
>  		kfree(to_cxl_root(port));
> -	else
> +	} else {
> +		put_device(dev->parent);
>  		kfree(port);
> +	}
>  }
>  
>  static ssize_t decoders_committed_show(struct device *dev,
> @@ -721,6 +724,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

v4 applied to cxl/fixes
19d2f0b97a131198efc2c4ca3eb7f980bba8c2b4