libnvdimm/security: Require nvdimm_security_setup_events() to succeed

The following warning:

    ACPI0012:00: security event setup failed: -19

...is meant to capture exceptional failures of sysfs_get_dirent(),
however it will also fail in the common case when security support is
disabled. A few issues:

1/ A dev_warn() report for a common case is too chatty
2/ The setup of this notifier is generic, no need for it to be driven
   from the nfit driver, it can exist completely in the core.
3/ If it fails for any reason besides security support being disabled,
   that's fatal and should abort DIMM activation. Userspace may hang if
   it never gets overwrite notifications.
4/ The dirent needs to be released.

Move the call to the core 'dimm' driver, make it conditional on security
support being active, make it fatal for the exceptional case, add the
missing sysfs_put() at device disable time.

Fixes: 7d988097c546 ("...Add security DSM overwrite support")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5143e11e3b0fa5378a5f08ee094706b92ba41962..c1fb06654749d624a6b44c26dab544fb582ca789 100644 (file)
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2042,11 +2042,6 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
                if (!nvdimm)
                        continue;
 
-               rc = nvdimm_security_setup_events(nvdimm);
-               if (rc < 0)
-                       dev_warn(acpi_desc->dev,
-                               "security event setup failed: %d\n", rc);
-
                nfit_kernfs = sysfs_get_dirent(nvdimm_kobj(nvdimm)->sd, "nfit");
                if (nfit_kernfs)
                        nfit_mem->flags_attr = sysfs_get_dirent(nfit_kernfs,

diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 0cf58cabc9eda43aed6a006a88b4f18fc88daa18..3cf50274fadb023b485fa613954b5b51af83240f 100644 (file)
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -26,6 +26,12 @@ static int nvdimm_probe(struct device *dev)
        struct nvdimm_drvdata *ndd;
        int rc;
 
+       rc = nvdimm_security_setup_events(dev);
+       if (rc < 0) {
+               dev_err(dev, "security event setup failed: %d\n", rc);
+               return rc;
+       }
+
        rc = nvdimm_check_config_data(dev);
        if (rc) {
                /* not required for non-aliased nvdimm, ex. NVDIMM-N */

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 4890310df87440a3fcd459de19d563e026c0922d..efe412a6b5b916220a1313a7ac194ce10f5232ca 100644 (file)
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -578,13 +578,25 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 }
 EXPORT_SYMBOL_GPL(__nvdimm_create);
 
-int nvdimm_security_setup_events(struct nvdimm *nvdimm)
+static void shutdown_security_notify(void *data)
 {
-       nvdimm->sec.overwrite_state = sysfs_get_dirent(nvdimm->dev.kobj.sd,
-                       "security");
+       struct nvdimm *nvdimm = data;
+
+       sysfs_put(nvdimm->sec.overwrite_state);
+}
+
+int nvdimm_security_setup_events(struct device *dev)
+{
+       struct nvdimm *nvdimm = to_nvdimm(dev);
+
+       if (nvdimm->sec.state < 0 || !nvdimm->sec.ops
+                       || !nvdimm->sec.ops->overwrite)
+               return 0;
+       nvdimm->sec.overwrite_state = sysfs_get_dirent(dev->kobj.sd, "security");
        if (!nvdimm->sec.overwrite_state)
-               return -ENODEV;
-       return 0;
+               return -ENOMEM;
+
+       return devm_add_action_or_reset(dev, shutdown_security_notify, nvdimm);
 }
 EXPORT_SYMBOL_GPL(nvdimm_security_setup_events);
 

diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index cfde992684e7db07de208b94b0f4b382e59701d3..379bf4305e6159a2568940df7cb06f07f774b55c 100644 (file)
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -250,6 +250,7 @@ long nvdimm_clear_poison(struct device *dev, phys_addr_t phys,
 void nvdimm_set_aliasing(struct device *dev);
 void nvdimm_set_locked(struct device *dev);
 void nvdimm_clear_locked(struct device *dev);
+int nvdimm_security_setup_events(struct device *dev);
 #if IS_ENABLED(CONFIG_NVDIMM_KEYS)
 int nvdimm_security_unlock(struct device *dev);
 #else

diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 7315977b64dacc4bf02b6cc925d5c80b3055d7f3..ad609617aeb87063ecb4fa8090cdced8df8dc74d 100644 (file)
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -235,7 +235,6 @@ static inline struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus,
                        cmd_mask, num_flush, flush_wpq, NULL, NULL);
 }
 
-int nvdimm_security_setup_events(struct nvdimm *nvdimm);
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd);
 const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd);
 u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,