Re: [PATCH v2 05/13] cxl/core: Restore the HDM decoders below DPort

[sashiko-bot@kernel.org]

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

New issues:
- [High] Lockless iteration of `port->endpoints` using `xa_for_each` in `cxl_port_recommit_decoders()` lacks protection from concurrent endpoint removal, leading to Use-After-Free.
- [Low] Unused local variable 'dport' whose address is passed to a function but never read.

Pre-existing issues:
- [High] Returning cxl_dport from find_cxl_port() leads to use-after-free because its lifecycle is managed by devres and is not tied to the port's reference count.
--

--- Patch [5]: [PATCH v2 05/13] cxl/core: Restore the HDM decoders below DPort ---
Due to system constraints regarding recitation of copyrighted or proprietary
material, this review is provided as a free-form summary without direct
inline quotes of the patch code.

commit c3d66df5869718f0ad0a7763a605cb85c574d23c
Author: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>

cxl/core: Restore the HDM decoders below DPort

This commit adds cxl_sbr_recommit_decoders() to restore HDM decoder settings
for CXL components below a Downstream Port after a Secondary Bus Reset. It
reads the endpoint set and switch decoder target lists while holding the
cxl_rwsem.region lock. It also drops the static declaration of find_cxl_port()
so it can be used outside of port.c.

[Severity: Low]
In drivers/cxl/core/dport_sbr.c, inside cxl_sbr_recommit_decoders(), the local
variable dport is declared and its address is passed to find_cxl_port(). Since
dport is never read or accessed again after this call, is this variable
necessary, or can we pass NULL if the downstream port is not needed?

[Severity: High]
In drivers/cxl/core/hdm.c, cxl_port_recommit_decoders() uses xa_for_each() to
iterate over port->endpoints. Does this lack protection from concurrent
endpoint removal?

The lock held during this walk is cxl_rwsem.region. Concurrently, a sysfs
unbind could trigger cxl_detach_ep(), which holds device_lock(&port->dev)
and frees the port_ep structure via kfree(). Because xa_for_each() internally
acquires and drops the RCU read lock per iteration, the port_ep pointer can
become dangling before the loop body dereferences port_ep->ep, resulting in a
use-after-free.

[Severity: High]
In drivers/cxl/core/port.c, find_cxl_port() populates and returns the dport
pointer via the cxl_find_port_ctx struct.

This is a pre-existing issue, but does retrieving the cxl_dport pointer this
way lead to a potential use-after-free? The cxl_dport memory is managed by
devres and is freed when the port's host device unbinds. It is not pinned by
the returned cxl_port's reference count. If a caller holds the port reference
but the port's host unbinds concurrently, accessing the dport pointer will
result in a use-after-free. Exporting find_cxl_port() to the rest of the
subsystem expands this vulnerable API surface.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825022628.3651434-1-fabio.m.de.francesco@linux.intel.com?part=5