lkml.org 
[lkml]   [2019]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] libnvdimm/security: Require nvdimm_security_setup_events() to succeed
From
Date

On 1/19/2019 10:58 AM, Dan Williams wrote:
> 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.
>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
> drivers/acpi/nfit/core.c | 5 -----
> drivers/nvdimm/dimm.c | 6 ++++++
> drivers/nvdimm/dimm_devs.c | 22 +++++++++++++++++-----
> drivers/nvdimm/nd.h | 1 +
> include/linux/libnvdimm.h | 1 -
> 5 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 9c95b82e5e5d..c38c914d8a58 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2073,11 +2073,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 0cf58cabc9ed..3cf50274fadb 100644
> --- 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 4890310df874..efe412a6b5b9 100644
> --- 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 cfde992684e7..379bf4305e61 100644
> --- 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 7315977b64da..ad609617aeb8 100644
> --- 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,
>

\
 
 \ /
  Last update: 2019-01-21 19:03    [W:0.039 / U:1.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site