[PATCH v3 2/2] cxl/region: Fix a race bug in delete_region_store()

[Sungwoo Kim]

This patch fixes race conditions in cxl region unregistration by (1) using
remove_devm_actions() instead of directly calling devm_release_action()
and (2) making unregister_region() idempotent.

Race:

CPU 0                     CPU 1
======                    ======
delete_region_store()
  cxlr = cxl_find_region_by_name()
                          delete_region_store()
                            cxlr = cxl_find_region_by_name()
  devm_release_action()
                            devm_release_action()
                            // cannot find the action, WARN_ON()

Splat:

WARNING: drivers/base/devres.c:824 at devm_release_action drivers/base/devres.c:824 [inline], CPU#0: syz.1.12224/47589
WARNING: drivers/base/devres.c:824 at devm_release_action+0x2b2/0x360 drivers/base/devres.c:817, CPU#0: syz.1.12224/47589

It's found by a fuzzer based on Syzkaller that rapidly executes CXL's
sysfs interface with QEMU CXL devices cxl-type3 and pxb-cxl.

Syzkaller's CXL grammar is available at:
https://github.com/swkim101/syz-cxl/blob/main/cxl.txt

The bug's impact seems low, since the region has already been released.
So, it does not affect users, except for a negligible warning message.

Fixes: 779dd20cfb56 ("cxl/region: Add region creation support")
Suggested-by: Dan Williams <djbw@xxxxxxxxxx>
Signed-off-by: Sungwoo Kim <iam@xxxxxxxxxxxx>
---
 drivers/cxl/core/region.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b086ae88b5bb..08b93b14a2c7 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2544,6 +2544,9 @@ static void unregister_region(void *_cxlr)
 	struct cxl_region_params *p = &cxlr->params;
 	int i;
 
+	if (test_and_set_bit(CXL_REGION_F_UNREGISTER, &cxlr->flags))
+		return;
+
 	device_del(&cxlr->dev);
 
 	/*
@@ -2863,15 +2866,18 @@ static ssize_t delete_region_store(struct device *dev,
 				   const char *buf, size_t len)
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
-	struct cxl_port *port = to_cxl_port(dev->parent);
 	struct cxl_region *cxlr;
 
+	/* remove_devm_actions_work() will put cxlr->dev. */
 	cxlr = cxl_find_region_by_name(cxlrd, buf);
 	if (IS_ERR(cxlr))
 		return PTR_ERR(cxlr);
 
-	devm_release_action(port->uport_dev, unregister_region, cxlr);
-	put_device(&cxlr->dev);
+	unregister_region(cxlr);
+	if (!remove_devm_actions(cxlr)) {
+		put_device(&cxlr->dev);
+		return -EBUSY;
+	}
 
 	return len;
 }
@@ -3736,7 +3742,6 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 {
 	struct cxl_endpoint_decoder *cxled = ctx->cxled;
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
-	struct cxl_port *port = cxlrd_to_port(cxlrd);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	int rc, part = READ_ONCE(cxled->part);
 	struct cxl_region *cxlr;
@@ -3757,7 +3762,12 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 
 	rc = __construct_region(cxlr, ctx);
 	if (rc) {
-		devm_release_action(port->uport_dev, unregister_region, cxlr);
+		/* remove_devm_actions_work() will put the device */
+		get_device(&cxlr->dev);
+		unregister_region(cxlr);
+		if (!remove_devm_actions(cxlr))
+			put_device(&cxlr->dev);
+
 		return ERR_PTR(rc);
 	}
 
-- 
2.47.3