summaryrefslogtreecommitdiff
path: root/drivers/nvdimm/security.c
diff options
context:
space:
mode:
authorDan Williams <dan.j.williams@intel.com>2019-08-27 03:54:54 +0300
committerDan Williams <dan.j.williams@intel.com>2019-08-29 23:49:13 +0300
commitd78c620a2e824d7b01a6e991208a8aa2c938cabe (patch)
treec37be9e345e45b7370fc4b0287472a478b790b7d /drivers/nvdimm/security.c
parent2b90cb223320a93b1be6c2616efe6f9ff14d8b28 (diff)
downloadlinux-d78c620a2e824d7b01a6e991208a8aa2c938cabe.tar.xz
libnvdimm/security: Introduce a 'frozen' attribute
In the process of debugging a system with an NVDIMM that was failing to unlock it was found that the kernel is reporting 'locked' while the DIMM security interface is 'frozen'. Unfortunately the security state is tracked internally as an enum which prevents it from communicating the difference between 'locked' and 'locked + frozen'. It follows that the enum also prevents the kernel from communicating 'unlocked + frozen' which would be useful for debugging why security operations like 'change passphrase' are disabled. Ditch the security state enum for a set of flags and introduce a new sysfs attribute explicitly for the 'frozen' state. The regression risk is low because the 'frozen' state was already blocked behind the 'locked' state, but will need to revisit if there were cases where applications need 'frozen' to show up in the primary 'security' attribute. The expectation is that communicating 'frozen' is mostly a helper for debug and status monitoring. Reviewed-by: Dave Jiang <dave.jiang@intel.com> Reported-by: Jeff Moyer <jmoyer@redhat.com> Reviewed-by: Jeff Moyer <jmoyer@redhat.com> Link: https://lore.kernel.org/r/156686729474.184120.5835135644278860826.stgit@dwillia2-desk3.amr.corp.intel.com Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Diffstat (limited to 'drivers/nvdimm/security.c')
-rw-r--r--drivers/nvdimm/security.c99
1 files changed, 47 insertions, 52 deletions
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index a570f2263a42..5862d0eee9db 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -158,7 +158,7 @@ static int nvdimm_key_revalidate(struct nvdimm *nvdimm)
}
nvdimm_put_key(key);
- nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
return 0;
}
@@ -174,7 +174,7 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return -EIO;
if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
@@ -189,7 +189,7 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
* freeze of the security configuration. I.e. if the OS does not
* have the key, security is being managed pre-OS.
*/
- if (nvdimm->sec.state == NVDIMM_SECURITY_UNLOCKED) {
+ if (test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.flags)) {
if (!key_revalidate)
return 0;
@@ -202,7 +202,7 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
rc == 0 ? "success" : "fail");
nvdimm_put_key(key);
- nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
return rc;
}
@@ -217,6 +217,24 @@ int nvdimm_security_unlock(struct device *dev)
return rc;
}
+static int check_security_state(struct nvdimm *nvdimm)
+{
+ struct device *dev = &nvdimm->dev;
+
+ if (test_bit(NVDIMM_SECURITY_FROZEN, &nvdimm->sec.flags)) {
+ dev_dbg(dev, "Incorrect security state: %#lx\n",
+ nvdimm->sec.flags);
+ return -EIO;
+ }
+
+ if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
+ dev_dbg(dev, "Security operation in progress.\n");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
{
struct device *dev = &nvdimm->dev;
@@ -229,19 +247,12 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
if (!nvdimm->sec.ops || !nvdimm->sec.ops->disable
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return -EOPNOTSUPP;
- if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
- dev_dbg(dev, "Incorrect security state: %d\n",
- nvdimm->sec.state);
- return -EIO;
- }
-
- if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
- dev_dbg(dev, "Security operation in progress.\n");
- return -EBUSY;
- }
+ rc = check_security_state(nvdimm);
+ if (rc)
+ return rc;
data = nvdimm_get_user_key_payload(nvdimm, keyid,
NVDIMM_BASE_KEY, &key);
@@ -253,7 +264,7 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
rc == 0 ? "success" : "fail");
nvdimm_put_key(key);
- nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
return rc;
}
@@ -271,14 +282,12 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
if (!nvdimm->sec.ops || !nvdimm->sec.ops->change_key
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return -EOPNOTSUPP;
- if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
- dev_dbg(dev, "Incorrect security state: %d\n",
- nvdimm->sec.state);
- return -EIO;
- }
+ rc = check_security_state(nvdimm);
+ if (rc)
+ return rc;
data = nvdimm_get_user_key_payload(nvdimm, keyid,
NVDIMM_BASE_KEY, &key);
@@ -301,10 +310,10 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
nvdimm_put_key(newkey);
nvdimm_put_key(key);
if (pass_type == NVDIMM_MASTER)
- nvdimm->sec.ext_state = nvdimm_security_state(nvdimm,
+ nvdimm->sec.ext_flags = nvdimm_security_flags(nvdimm,
NVDIMM_MASTER);
else
- nvdimm->sec.state = nvdimm_security_state(nvdimm,
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm,
NVDIMM_USER);
return rc;
}
@@ -322,7 +331,7 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
if (!nvdimm->sec.ops || !nvdimm->sec.ops->erase
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return -EOPNOTSUPP;
if (atomic_read(&nvdimm->busy)) {
@@ -330,18 +339,11 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
return -EBUSY;
}
- if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
- dev_dbg(dev, "Incorrect security state: %d\n",
- nvdimm->sec.state);
- return -EIO;
- }
-
- if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
- dev_dbg(dev, "Security operation in progress.\n");
- return -EBUSY;
- }
+ rc = check_security_state(nvdimm);
+ if (rc)
+ return rc;
- if (nvdimm->sec.ext_state != NVDIMM_SECURITY_UNLOCKED
+ if (!test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.ext_flags)
&& pass_type == NVDIMM_MASTER) {
dev_dbg(dev,
"Attempt to secure erase in wrong master state.\n");
@@ -359,7 +361,7 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
rc == 0 ? "success" : "fail");
nvdimm_put_key(key);
- nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
return rc;
}
@@ -375,7 +377,7 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
if (!nvdimm->sec.ops || !nvdimm->sec.ops->overwrite
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return -EOPNOTSUPP;
if (atomic_read(&nvdimm->busy)) {
@@ -388,16 +390,9 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
return -EINVAL;
}
- if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
- dev_dbg(dev, "Incorrect security state: %d\n",
- nvdimm->sec.state);
- return -EIO;
- }
-
- if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
- dev_dbg(dev, "Security operation in progress.\n");
- return -EBUSY;
- }
+ rc = check_security_state(nvdimm);
+ if (rc)
+ return rc;
data = nvdimm_get_user_key_payload(nvdimm, keyid,
NVDIMM_BASE_KEY, &key);
@@ -412,7 +407,7 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
if (rc == 0) {
set_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags);
set_bit(NDD_WORK_PENDING, &nvdimm->flags);
- nvdimm->sec.state = NVDIMM_SECURITY_OVERWRITE;
+ set_bit(NVDIMM_SECURITY_OVERWRITE, &nvdimm->sec.flags);
/*
* Make sure we don't lose device while doing overwrite
* query.
@@ -443,7 +438,7 @@ void __nvdimm_security_overwrite_query(struct nvdimm *nvdimm)
tmo = nvdimm->sec.overwrite_tmo;
if (!nvdimm->sec.ops || !nvdimm->sec.ops->query_overwrite
- || nvdimm->sec.state < 0)
+ || !nvdimm->sec.flags)
return;
rc = nvdimm->sec.ops->query_overwrite(nvdimm);
@@ -467,8 +462,8 @@ void __nvdimm_security_overwrite_query(struct nvdimm *nvdimm)
clear_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags);
clear_bit(NDD_WORK_PENDING, &nvdimm->flags);
put_device(&nvdimm->dev);
- nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
- nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, NVDIMM_MASTER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
+ nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_MASTER);
}
void nvdimm_security_overwrite_query(struct work_struct *work)