The blanket blocking of all security operations while the DIMM is in
active use in a region is too restrictive. The only security operations
that need to be aware of the ->busy state are those that mutate the
state of data, i.e. erase and overwrite.
Refactor the ->busy checks to be applied at the entry common entry point
in __security_store() rather than each of the helper routines.
Cc: Dave Jiang <dave.ji...@intel.com>
Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
---
drivers/nvdimm/dimm_devs.c | 33 ++++++++++++++++-----------------
drivers/nvdimm/security.c | 10 ----------
2 files changed, 16 insertions(+), 27 deletions(-)
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 53330625fe07..d837cb9be83d 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -424,9 +424,6 @@ static ssize_t __security_store(struct device *dev, const
char *buf, size_t len)
unsigned int key, newkey;
int i;
- if (atomic_read(&nvdimm->busy))
- return -EBUSY;
-
rc = sscanf(buf, "%"__stringify(SEC_CMD_SIZE)"s"
" %"__stringify(KEY_ID_SIZE)"s"
" %"__stringify(KEY_ID_SIZE)"s",
@@ -451,23 +448,25 @@ static ssize_t __security_store(struct device *dev, const
char *buf, size_t len)
} else if (i == OP_DISABLE) {
dev_dbg(dev, "disable %u\n", key);
rc = nvdimm_security_disable(nvdimm, key);
- } else if (i == OP_UPDATE) {
- dev_dbg(dev, "update %u %u\n", key, newkey);
- rc = nvdimm_security_update(nvdimm, key, newkey, NVDIMM_USER);
- } else if (i == OP_ERASE) {
- dev_dbg(dev, "erase %u\n", key);
- rc = nvdimm_security_erase(nvdimm, key, NVDIMM_USER);
+ } else if (i == OP_UPDATE || i == OP_MASTER_UPDATE) {
+ dev_dbg(dev, "%s %u %u\n", ops[i].name, key, newkey);
+ rc = nvdimm_security_update(nvdimm, key, newkey, i == OP_UPDATE
+ ? NVDIMM_USER : NVDIMM_MASTER);
+ } else if (i == OP_ERASE || i == OP_MASTER_ERASE) {
+ dev_dbg(dev, "%s %u\n", ops[i].name, key);
+ if (atomic_read(&nvdimm->busy)) {
+ dev_dbg(dev, "Unable to secure erase while DIMM
active.\n");
+ return -EBUSY;
+ }
+ rc = nvdimm_security_erase(nvdimm, key, i == OP_ERASE
+ ? NVDIMM_USER : NVDIMM_MASTER);
} else if (i == OP_OVERWRITE) {
dev_dbg(dev, "overwrite %u\n", key);
+ if (atomic_read(&nvdimm->busy)) {
+ dev_dbg(dev, "Unable to overwrite while DIMM
active.\n");
+ return -EBUSY;
+ }
rc = nvdimm_security_overwrite(nvdimm, key);
- } else if (i == OP_MASTER_UPDATE) {
- dev_dbg(dev, "master_update %u %u\n", key, newkey);
- rc = nvdimm_security_update(nvdimm, key, newkey,
- NVDIMM_MASTER);
- } else if (i == OP_MASTER_ERASE) {
- dev_dbg(dev, "master_erase %u\n", key);
- rc = nvdimm_security_erase(nvdimm, key,
- NVDIMM_MASTER);
} else
return -EINVAL;
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 5862d0eee9db..2166e627383a 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -334,11 +334,6 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned
int keyid,
|| !nvdimm->sec.flags)
return -EOPNOTSUPP;
- if (atomic_read(&nvdimm->busy)) {
- dev_dbg(dev, "Unable to secure erase while DIMM active.\n");
- return -EBUSY;
- }
-
rc = check_security_state(nvdimm);
if (rc)
return rc;
@@ -380,11 +375,6 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm,
unsigned int keyid)
|| !nvdimm->sec.flags)
return -EOPNOTSUPP;
- if (atomic_read(&nvdimm->busy)) {
- dev_dbg(dev, "Unable to overwrite while DIMM active.\n");
- return -EBUSY;
- }
-
if (dev->driver == NULL) {
dev_dbg(dev, "Unable to overwrite while DIMM active.\n");
return -EINVAL;
