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

[Guixin Liu <kanie@linux.alibaba.com>]

在 2026/8/28 08:33, Alison Schofield 写道:
> 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
Sure, I will cut the commit body,  and explain how I test it.

Best Regards,
Guixin Liu