[PATCH v2] cxl/memdev: Decouple security init from devm_cxl_add_memdev()

[Davidlohr Bueso <dave@stgolabs.net>]

A crash was reported during type2 device enablement[0] which called
devm_cxl_add_memdev() without an mds, causing the security state
init steps to be bogus.

  BUG: kernel NULL pointer dereference, address: 0000000000000278
  [...]
  RIP: 0010:devm_cxl_add_memdev+0x1de/0x2c0 [cxl_core]
  [...]
  Call Trace:
   <TASK>
   ? __die+0x1f/0x70
   ? page_fault_oops+0x149/0x420
   ? fixup_exception+0x22/0x310
   ? kernelmode_fixup_or_oops+0x84/0x110
   ? exc_page_fault+0x6d/0x150
   ? asm_exc_page_fault+0x22/0x30
   ? devm_cxl_add_memdev+0x1de/0x2c0 [cxl_core]
   cxl_mock_mem_probe+0x632/0x870 [cxl_mock_mem]
   platform_probe+0x40/0x90
   really_probe+0x19e/0x3e0
   ? __pfx___driver_attach+0x10/0x10
   __driver_probe_device+0x78/0x160
   driver_probe_device+0x1f/0x90
   __driver_attach+0xce/0x1c0
   bus_for_each_dev+0x63/0xa0
   bus_add_driver+0x112/0x210
   driver_register+0x55/0x100
   ? __pfx_cxl_mock_mem_driver_init+0x10/0x10 [cxl_mock_mem]
   [...]

Move out cxl_memdev_security_init() and allow the pci probing to call
it directly. This is more aligned with the intention of f6b8ab32e3ec
("cxl/memdev: Make mailbox functionality optional").

The cxl_memdev_security_shutdown() counterpart is removed altogether
and handle the sanitation corner case directly in the unregister. In
addition, the devm_cxl_add_memdev() cleanup path case is about ioctls,
and regardless, there is no way for the sanitation to become active
during that time.

[0] https://lore.kernel.org/all/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/

Reported-by: Ira Weiny <ira.weiny@intel.com>
Tested-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Changes from v1:
 - Picked up tags.
 - Add missing EXPORT_SYMBOL_NS_GPL.
 - Kill the shutdown function.
 - Fix typo in changelog.

 drivers/cxl/core/memdev.c | 28 +++++++++-------------------
 drivers/cxl/cxlmem.h      |  1 +
 drivers/cxl/pci.c         |  4 ++++
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 14b547c07f54..13d334b11e64 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -556,21 +556,11 @@ void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 }
 EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, CXL);
 
-static void cxl_memdev_security_shutdown(struct device *dev)
-{
-	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
-	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
-
-	if (mds->security.poll)
-		cancel_delayed_work_sync(&mds->security.poll_dwork);
-}
-
 static void cxl_memdev_shutdown(struct device *dev)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
 
 	down_write(&cxl_memdev_rwsem);
-	cxl_memdev_security_shutdown(dev);
 	cxlmd->cxlds = NULL;
 	up_write(&cxl_memdev_rwsem);
 }
@@ -579,6 +569,10 @@ static void cxl_memdev_unregister(void *_cxlmd)
 {
 	struct cxl_memdev *cxlmd = _cxlmd;
 	struct device *dev = &cxlmd->dev;
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	if (mds && mds->security.poll)
+		cancel_delayed_work_sync(&mds->security.poll_dwork);
 
 	cxl_memdev_shutdown(dev);
 	cdev_device_del(&cxlmd->cdev, dev);
@@ -1009,11 +1003,10 @@ static void put_sanitize(void *data)
 	sysfs_put(mds->security.sanitize_node);
 }
 
-static int cxl_memdev_security_init(struct cxl_memdev *cxlmd)
+int cxl_memdev_security_state_init(struct cxl_memdev_state *mds)
 {
-	struct cxl_dev_state *cxlds = cxlmd->cxlds;
-	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
-	struct device *dev = &cxlmd->dev;
+	struct cxl_dev_state *cxlds = &mds->cxlds;
+	struct device *dev = &cxlds->cxlmd->dev;
 	struct kernfs_node *sec;
 
 	sec = sysfs_get_dirent(dev->kobj.sd, "security");
@@ -1029,7 +1022,8 @@ static int cxl_memdev_security_init(struct cxl_memdev *cxlmd)
 	}
 
 	return devm_add_action_or_reset(cxlds->dev, put_sanitize, mds);
- }
+}
+EXPORT_SYMBOL_NS_GPL(cxl_memdev_security_state_init, CXL);
 
 struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 {
@@ -1059,10 +1053,6 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
 	if (rc)
 		goto err;
 
-	rc = cxl_memdev_security_init(cxlmd);
-	if (rc)
-		goto err;
-
 	rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister, cxlmd);
 	if (rc)
 		return ERR_PTR(rc);
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 706f8a6d1ef4..b99099eb2402 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -884,6 +884,7 @@ static inline void cxl_mem_active_dec(void)
 #endif
 
 int cxl_mem_sanitize(struct cxl_memdev_state *mds, u16 cmd);
+int cxl_memdev_security_state_init(struct cxl_memdev_state *mds);
 
 struct cxl_hdm {
 	struct cxl_component_regs regs;
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 1cb1494c28fe..5242dbf0044d 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -887,7 +887,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
 
+	rc = cxl_memdev_security_state_init(mds);
+	if (rc)
+		return rc;
+
 	rc = cxl_memdev_setup_fw_upload(mds);
 	if (rc)
 		return rc;
--
2.41.0