[PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection

[PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection

[Guixin Liu]

__cxl_endpoint_decoder_reset_detected() samples the Committed bit of an
enabled endpoint decoder by reading the decoder control register at
cxlhdm->regs.hdm_decoder, without testing that pointer.
should_emulate_decoders(), the other consumer of the same field, does test
it, because the field is optional.

A memory device that exposes no component registers and describes its HDM
ranges through the CXL DVSEC range registers takes the early return in
devm_cxl_setup_hdm(), which leaves regs.hdm_decoder NULL and derives the
decoder count from the DVSEC ranges instead.
cxl_setup_hdm_decoder_from_dvsec() then publishes those emulated decoders
with CXL_DECODER_F_ENABLE set, and CXL_DECODER_F_ENABLE is the only filter
cxl_reset_done() applies when it walks the endpoint's decoders after an FLR
or an SBR. So on such a device every reset reaches the readl() with a NULL
base and oopses in the PCI reset completion path.

Report "no reset detected" when the endpoint has no HDM decoder registers,
as should_emulate_decoders() already does for the same condition. A range
described by the DVSEC registers has no Committed bit that a reset could
clear, so there is nothing for the post-reset warning to observe.

Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
This was patch 4/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:
- 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/pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 9d807c1a002c..611175f29d24 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -685,6 +685,14 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
 
 	cxlhdm = dev_get_drvdata(&port->dev);
 	hdm = cxlhdm->regs.hdm_decoder;
+
+	/*
+	 * Devices that describe their HDM ranges with the DVSEC range
+	 * registers have no HDM decoder registers to consult.
+	 */
+	if (!hdm)
+		return 0;
+
 	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
 
 	return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl);

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7

Re: [PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete fix for DVSEC-emulated decoders allows spurious reset warnings and kernel tainting.
- [High] Race condition between PCIe reset and sysfs driver unbind leading to NULL pointer dereference or UAF.
- [High] Data race on `cxld->flags` due to missing synchronization.
--

--- Patch [1]: [PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection ---
commit 0a3a66bfcbdad33db13f3c33dd88b689403ca622
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/pci: Fix NULL pointer dereference in reset detection

This commit adds a check for a NULL hdm pointer before reading the control
register in __cxl_endpoint_decoder_reset_detected(). This prevents a kernel
oops during PCIe resets on memory devices that emulate their HDM decoders
using DVSEC ranges instead of physical registers.

Link: https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/

> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002ca..611175f29d245 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -685,6 +685,14 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
>  
>  	cxlhdm = dev_get_drvdata(&port->dev);
>  	hdm = cxlhdm->regs.hdm_decoder;

[Severity: High]
This is a pre-existing issue, but is there a race condition here between
the PCIe reset handling and sysfs driver unbind?

Looking at cxl_reset_done(), it iterates over the endpoint's children using
device_for_each_child() without holding the port's device lock:

drivers/cxl/pci.c:cxl_reset_done() {
    ...
    device_for_each_child(&cxlmd->endpoint->dev, NULL,
                          cxl_endpoint_decoder_clear_reset_flags);
    ...
}

If a concurrent sysfs unbind of the cxl_port driver clears the driver data
and frees the cxlhdm structure, could dev_get_drvdata() return NULL or a
stale pointer, leading to a crash when cxlhdm is dereferenced?

> +
> +	/*
> +	 * Devices that describe their HDM ranges with the DVSEC range
> +	 * registers have no HDM decoder registers to consult.
> +	 */
> +	if (!hdm)
> +		return 0;

[Severity: High]
This isn't a bug introduced by this patch, but does this check fully cover
all DVSEC-emulated decoders?

In should_emulate_decoders(), a device can fall back to DVSEC emulation
even when the hdm pointer is not NULL (for example, if HDM decoders are
present but globally disabled by firmware).

In this fallback case, the decoder is emulated but the hdm pointer remains
valid. This function would bypass the !hdm check, read the HDM control
register where the COMMITTED bit is likely 0, and falsely trigger a reset
warning.

Since cxl_setup_hdm_decoder_from_dvsec() sets cxld->commit to NULL for all
emulated decoders, would checking for emulation directly (e.g.,
if (!cxld->commit)) be a more robust approach?

> +
>  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
>  
>  	return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl);
>  }

[Severity: High]
This is a pre-existing issue, but does modifying cxld->flags during reset
handling lack necessary synchronization?

In cxl_endpoint_decoder_clear_reset_flags(), which is called during
cxl_reset_done(), the flags are modified using a non-atomic operation:

drivers/cxl/pci.c:cxl_endpoint_decoder_clear_reset_flags() {
    ...
    cxled->cxld.flags &= ~CXL_DECODER_F_RESET_MASK;
    ...
}

This happens while holding only the parent memory device lock.
Concurrently, sysfs operations can commit a decoder:

cxl_decoder_commit() {
    ...
    cxld->flags |= CXL_DECODER_F_ENABLE;
    ...
}

Because cxld->flags is modified concurrently without a common lock or atomic
bit operations, could flag updates be lost, potentially causing the loss of
the ENABLE flag?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812061027.57037-1-kanie@linux.alibaba.com?part=1

Re: [PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection

[Richard Cheng]

On Wed, Aug 12, 2026 at 02:10:27PM +0800, Guixin Liu wrote:
> __cxl_endpoint_decoder_reset_detected() samples the Committed bit of an
> enabled endpoint decoder by reading the decoder control register at
> cxlhdm->regs.hdm_decoder, without testing that pointer.
> should_emulate_decoders(), the other consumer of the same field, does test
> it, because the field is optional.
> 
> A memory device that exposes no component registers and describes its HDM
> ranges through the CXL DVSEC range registers takes the early return in
> devm_cxl_setup_hdm(), which leaves regs.hdm_decoder NULL and derives the
> decoder count from the DVSEC ranges instead.
> cxl_setup_hdm_decoder_from_dvsec() then publishes those emulated decoders
> with CXL_DECODER_F_ENABLE set, and CXL_DECODER_F_ENABLE is the only filter
> cxl_reset_done() applies when it walks the endpoint's decoders after an FLR
> or an SBR. So on such a device every reset reaches the readl() with a NULL
> base and oopses in the PCI reset completion path.
> 
> Report "no reset detected" when the endpoint has no HDM decoder registers,
> as should_emulate_decoders() already does for the same condition. A range
> described by the DVSEC registers has no Committed bit that a reset could
> clear, so there is nothing for the post-reset warning to observe.
> 
> Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
> This was patch 4/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:
> - 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/pci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002c..611175f29d24 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -685,6 +685,14 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
>  
>  	cxlhdm = dev_get_drvdata(&port->dev);
>  	hdm = cxlhdm->regs.hdm_decoder;
> +
> +	/*
> +	 * Devices that describe their HDM ranges with the DVSEC range
> +	 * registers have no HDM decoder registers to consult.
> +	 */
> +	if (!hdm)
> +		return 0;
> +

Hi Guixin,

I think we should cover the case for DVSEC-enulated decoder as well, where hdm is not NULL.
That could happen when HDM registers exist but are globally disabled, so the driver uses DVSEC ranges instead.

In that case it will still pass your "if (!hdm)" check and reset detection still reads the unused HDM COMMITTED bit.

Maybe we should also check for "!cxld->commit" ?

Best regards,
Richard Cheng.

>  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
>  
>  	return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl);
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> -- 
> 2.43.7
> 
> 

Re: [PATCH v2] cxl/pci: Fix NULL pointer dereference in reset detection

[Guixin Liu]

在 2026/8/12 15:38, Richard Cheng 写道:
> Hi Guixin,
>
> I think we should cover the case for DVSEC-enulated decoder as well, where hdm is not NULL.
> That could happen when HDM registers exist but are globally disabled, so the driver uses DVSEC ranges instead.
>
> In that case it will still pass your "if (!hdm)" check and reset detection still reads the unused HDM COMMITTED bit.
>
> Maybe we should also check for "!cxld->commit" ?
>
> Best regards,
> Richard Cheng.
Yes, and just "!cxld->commit" is ok, this cover "!hdm". Best Regards, 
Guixin Liu