Re: [PATCH] cxl/hdm: Avoid NULL deref when component registers are missing

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 29 Mar 2023 14:35:43 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> The cxl_port driver attempts to support endpoint devices that do not
> advertise a component register block, but by inspection
> devm_cxl_setup_hdm() passes a NULL @crb to helper functions that should
> be skipped.
> 
> Return early and skip setting target_count since that is only relevant
> for switch decoders, not endpoint decoders.

This is a good observation. It would be nice to not read it for the
HDM decoder path either. Obviously we don't use it so that doesn't do
any harm, but to someone reading the code it looks like we care about the
value.  I'm not immediately sure how we'd establish at this layer that
the HDM decoder is a switch or HB one though..

> 
> Fixes: 757f6448b100 ("cxl/hdm: Fix double allocation of @cxlhdm")
> Tested-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Patch looks fine to me.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/hdm.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 5293fe13fce3..cc123996b1a4 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -123,7 +123,7 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>  	crb = ioremap(port->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);
>  	if (!crb && info && info->mem_enabled) {
>  		cxlhdm->decoder_count = info->ranges;
> -		cxlhdm->target_count = info->ranges;
> +		return cxlhdm;
>  	} else if (!crb) {
>  		dev_err(dev, "No component registers mapped\n");
>  		return ERR_PTR(-ENXIO);
>