Re: [PATCH v2] cxl/core: Fix dport use-after-free via the einj_inject debugfs file

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Aug 12, 2026 at 02:09:43PM +0800, Guixin Liu wrote:


Hi Guixin Liu,

Skipping ahead to the changelog, you write
> - rewrite the commit message to describe the behaviour rather than narrate
>   the code change (Alison Schofield)

I went back and looked at v1, and somehow the commit message got
substantially longer and harder to understand in response to my request
to make it clearer. V1 was actually closer. It needed to be simplified,
not expanded.

This reads more like a forensic report of everything learned while
investigating the bug than a commit message explaining why this change
is needed. It is dense material, far too detailed to the point of
clarifying nothing.



> cxl_debugfs_create_dport_dir() publishes a debugfs directory containing an
> "einj_inject" file whose i_private is the 'struct cxl_dport', then discards
> the returned dentry and registers nothing to remove it. The other per-dport
> facility set up next to it in __devm_cxl_add_dport(),
> devm_cxl_dport_ras_setup(), binds its resources to dport_to_host(dport) so
> that they go away with the dport. The debugfs directory has no such owner:
> it lives until cxl_core is unloaded and cxl_core_exit() tears down the
> whole cxl/ tree.

Most of the above is unnecessary. The comparison with devm_cxl_dport_ras_setup()
adds nothing to the description of the bug. Neither does walking through the
eventual module cleanup.

The important background is simply that einj_inject retains a pointer
to the cxl_dport, but its lifetime is not tied to the dport.

> 
> The dport itself is freed much earlier. free_dport() is registered in the
> dport's devres group, so the dport is freed when the host device is
> unbound, which is an ordinary sysfs operation on the host bridge port or
> the ACPI0017 root, not a module-teardown-only path. After that unbind the
> einj_inject file is still there, and a write to it calls cxl_einj_inject()
> on freed memory, reading dport->rch and dport->dport_dev and passing them
> to the EINJ code.


Above, the important point is buried at the end. Unbinding the host frees
the dport but leaves einj_inject behind, so writing the file can
dereference the freed dport.

That is the issue and impact. The devres mechanics, specific devices that
can be unbound, and individual fields subsequently accessed do not make
that clearer.


> 
> Re-binding the topology does not recover either. The stale directory keeps
> the dport device's name, so the second creation finds the name in use,
> debugfs setup fails, and error injection is silently unavailable for that
> dport for the remaining lifetime of the module.


Above is useful as a secondary impact, but can be one sentence: the stale
directory also prevents the debugfs directory from being recreated on
rebind.


> 
> Keep the dentry and remove the directory from a devm action on the dport's
> host device. The action is registered after free_dport() within the same
> devres group, so release ordering runs it before the dport is freed. Its
> registration failure is deliberately not propagated, following
> devm_cxl_dport_ras_setup(): a missing debugfs directory is not a
> functional failure of the dport, and on that path
> devm_add_action_or_reset() has already removed the directory itself, so
> there is no dangling node left and nothing to gain from failing the dport
> addition.

This is implementation narration. The resolution is simply to remove
the per-dport debugfs directory when the dport host is released. If
there is a subtle implementation choice that needs explanation, put a
short comment where that choice is made.

If you want to give your LLM some direction, ask it to first reduce the
commit message to something like:

Background:
einj_inject retains a pointer to the cxl_dport.

Issue:
The debugfs file can outlive the dport.

Impact:
After unbind, writing einj_inject can dereference the freed dport.
The stale directory also prevents recreation on rebind.

Resolution:
Remove the per-dport debugfs directory when the dport host is
released.

Then turn that into a short, flowing commit message. Do not expand each
item with everything learned while investigating the bug.

Being able to summarize a change this way is important beyond making the
commit message easier to review. It also gives reviewers confidence that
the submitter understands the problem, its impact, and why the proposed
change fixes it, rather than simply forwarding an AI-generated analysis
and patch.

On that point, please also say how this issue was found. Was it found by
you, by an AI analysis tool, or by reproducing an actual failure?

Also, how was this tested? Did you actually unbind the host, access
einj_inject after the unbind, and reproduce the UAF? Did you then verify
that the patched kernel removes the debugfs entry and that it is
recreated after rebind? Please describe the actual test performed.

That information gives me considerably more confidence in the patch than
several paragraphs of forensic implementation detail.

-- Alison


> 
> Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
> This was patch 3/8 of the "cxl: Assorted fixes" series [1]. Per review
> feedback that series is not being reworked as a whole; the fixes are resent
> individually instead. Patches 1, 2 and 7 of the series are dropped, as those
> issues are already fixed in cxl/next.
> 
> v1->v2:
> - do not propagate the devm_add_action_or_reset() failure out of
>   cxl_debugfs_create_dport_dir(); a missing debugfs directory must not fail
>   the dport addition (Li Ming)
> - rebase onto cxl/next
> - rewrite the commit message to describe the behaviour rather than narrate
>   the code change (Alison Schofield)
> 
> [1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/
> 
>  drivers/cxl/core/port.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 625e4aa427db..62a9c2038d1f 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -814,6 +814,11 @@ static int cxl_einj_inject(void *data, u64 type)
>  DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
>  			 "0x%llx\n");
>  
> +static void remove_debugfs(void *dentry)
> +{
> +	debugfs_remove_recursive(dentry);
> +}
> +
>  static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>  {
>  	struct cxl_port *parent = parent_port_of(dport->port);
> @@ -834,6 +839,8 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>  
>  	debugfs_create_file("einj_inject", 0200, dir, dport,
>  			    &cxl_einj_inject_fops);
> +
> +	devm_add_action_or_reset(dport_to_host(dport), remove_debugfs, dir);
>  }
>  
>  static int cxl_port_add(struct cxl_port *port,
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> -- 
> 2.43.7
>