Commit 71ee71d7 authored by Vishal Verma's avatar Vishal Verma Committed by Dan Williams
Browse files

cxl/region: Fix decoder allocation crash 



When an intermediate port's decoders have been exhausted by existing
regions, and creating a new region with the port in question in it's
hierarchical path is attempted, cxl_port_attach_region() fails to find a
port decoder (as would be expected), and drops into the failure / cleanup
path.

However, during cleanup of the region reference, a sanity check attempts
to dereference the decoder, which in the above case didn't exist. This
causes a NULL pointer dereference BUG.

To fix this, refactor the decoder allocation and de-allocation into
helper routines, and in this 'free' routine, check that the decoder,
@cxld, is valid before attempting any operations on it.

Cc: <stable@vger.kernel.org>
Suggested-by: default avatarDan Williams <dan.j.williams@intel.com>
Signed-off-by: default avatarVishal Verma <vishal.l.verma@intel.com>
Reviewed-by: default avatarDave Jiang <dave.jiang@intel.com>
Fixes: 384e624b ("cxl/region: Attach endpoint decoders")
Link: https://lore.kernel.org/r/20221101074100.1732003-1-vishal.l.verma@intel.com


Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 24f0692b

drivers/cxl/core/region.c

+41 −26
Original line number Diff line number Diff line
@@ -687,18 +687,27 @@ static struct cxl_region_ref *alloc_region_ref(struct cxl_port *port, 
	return cxl_rr;

}



static void free_region_ref(struct cxl_region_ref *cxl_rr)

static void cxl_rr_free_decoder(struct cxl_region_ref *cxl_rr)

{

	struct cxl_port *port = cxl_rr->port;

	struct cxl_region *cxlr = cxl_rr->region;

	struct cxl_decoder *cxld = cxl_rr->decoder;



	if (!cxld)

		return;



	dev_WARN_ONCE(&cxlr->dev, cxld->region != cxlr, "region mismatch\n");

	if (cxld->region == cxlr) {

		cxld->region = NULL;

		put_device(&cxlr->dev);

	}

}



static void free_region_ref(struct cxl_region_ref *cxl_rr)

{

	struct cxl_port *port = cxl_rr->port;

	struct cxl_region *cxlr = cxl_rr->region;



	cxl_rr_free_decoder(cxl_rr);

	xa_erase(&port->regions, (unsigned long)cxlr);

	xa_destroy(&cxl_rr->endpoints);

	kfree(cxl_rr);
@@ -729,6 +738,33 @@ static int cxl_rr_ep_add(struct cxl_region_ref *cxl_rr, 
	return 0;

}



static int cxl_rr_alloc_decoder(struct cxl_port *port, struct cxl_region *cxlr,

				struct cxl_endpoint_decoder *cxled,

				struct cxl_region_ref *cxl_rr)

{

	struct cxl_decoder *cxld;



	if (port == cxled_to_port(cxled))

		cxld = &cxled->cxld;

	else

		cxld = cxl_region_find_decoder(port, cxlr);

	if (!cxld) {

		dev_dbg(&cxlr->dev, "%s: no decoder available\n",

			dev_name(&port->dev));

		return -EBUSY;

	}



	if (cxld->region) {

		dev_dbg(&cxlr->dev, "%s: %s already attached to %s\n",

			dev_name(&port->dev), dev_name(&cxld->dev),

			dev_name(&cxld->region->dev));

		return -EBUSY;

	}



	cxl_rr->decoder = cxld;

	return 0;

}



/**

 * cxl_port_attach_region() - track a region's interest in a port by endpoint

 * @port: port to add a new region reference 'struct cxl_region_ref'
@@ -795,12 +831,6 @@ static int cxl_port_attach_region(struct cxl_port *port, 
			cxl_rr->nr_targets++;

			nr_targets_inc = true;

		}



		/*

		 * The decoder for @cxlr was allocated when the region was first

		 * attached to @port.

		 */

		cxld = cxl_rr->decoder;

	} else {

		cxl_rr = alloc_region_ref(port, cxlr);

		if (IS_ERR(cxl_rr)) {
@@ -811,26 +841,11 @@ static int cxl_port_attach_region(struct cxl_port *port, 
		}

		nr_targets_inc = true;



		if (port == cxled_to_port(cxled))

			cxld = &cxled->cxld;

		else

			cxld = cxl_region_find_decoder(port, cxlr);

		if (!cxld) {

			dev_dbg(&cxlr->dev, "%s: no decoder available\n",

				dev_name(&port->dev));

			goto out_erase;

		}



		if (cxld->region) {

			dev_dbg(&cxlr->dev, "%s: %s already attached to %s\n",

				dev_name(&port->dev), dev_name(&cxld->dev),

				dev_name(&cxld->region->dev));

			rc = -EBUSY;

		rc = cxl_rr_alloc_decoder(port, cxlr, cxled, cxl_rr);

		if (rc)

			goto out_erase;

	}



		cxl_rr->decoder = cxld;

	}

	cxld = cxl_rr->decoder;



	rc = cxl_rr_ep_add(cxl_rr, cxled);

	if (rc) {