@@ -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);
@@ -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;
@@ -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;
--
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(-) 2.41.0