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

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 <[email protected]>
> ---
> 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/[email protected]/
> 
>  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
> 
>