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>
This commit is contained in:
Dan Williams 2019-08-26 17:54:54 -07:00
parent 2b90cb2233
commit d78c620a2e
7 changed files with 142 additions and 128 deletions

View File

@ -7,10 +7,11 @@
#include "intel.h"
#include "nfit.h"
static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm,
static unsigned long intel_security_flags(struct nvdimm *nvdimm,
enum nvdimm_passphrase_type ptype)
{
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
unsigned long security_flags = 0;
struct {
struct nd_cmd_pkg pkg;
struct nd_intel_get_security_state cmd;
@ -27,7 +28,7 @@ static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm,
int rc;
if (!test_bit(NVDIMM_INTEL_GET_SECURITY_STATE, &nfit_mem->dsm_mask))
return -ENXIO;
return 0;
/*
* Short circuit the state retrieval while we are doing overwrite.
@ -35,38 +36,42 @@ static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm,
* until the overwrite DSM completes.
*/
if (nvdimm_in_overwrite(nvdimm) && ptype == NVDIMM_USER)
return NVDIMM_SECURITY_OVERWRITE;
return BIT(NVDIMM_SECURITY_OVERWRITE);
rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
if (rc < 0)
return rc;
if (nd_cmd.cmd.status)
return -EIO;
if (rc < 0 || nd_cmd.cmd.status) {
pr_err("%s: security state retrieval failed (%d:%#x)\n",
nvdimm_name(nvdimm), rc, nd_cmd.cmd.status);
return 0;
}
/* check and see if security is enabled and locked */
if (ptype == NVDIMM_MASTER) {
if (nd_cmd.cmd.extended_state & ND_INTEL_SEC_ESTATE_ENABLED)
return NVDIMM_SECURITY_UNLOCKED;
else if (nd_cmd.cmd.extended_state &
ND_INTEL_SEC_ESTATE_PLIMIT)
return NVDIMM_SECURITY_FROZEN;
} else {
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_UNSUPPORTED)
return -ENXIO;
else if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_ENABLED) {
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_LOCKED)
return NVDIMM_SECURITY_LOCKED;
else if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_FROZEN
|| nd_cmd.cmd.state &
ND_INTEL_SEC_STATE_PLIMIT)
return NVDIMM_SECURITY_FROZEN;
else
return NVDIMM_SECURITY_UNLOCKED;
}
set_bit(NVDIMM_SECURITY_UNLOCKED, &security_flags);
else
set_bit(NVDIMM_SECURITY_DISABLED, &security_flags);
if (nd_cmd.cmd.extended_state & ND_INTEL_SEC_ESTATE_PLIMIT)
set_bit(NVDIMM_SECURITY_FROZEN, &security_flags);
return security_flags;
}
/* this should cover master security disabled as well */
return NVDIMM_SECURITY_DISABLED;
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_UNSUPPORTED)
return 0;
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_ENABLED) {
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_FROZEN ||
nd_cmd.cmd.state & ND_INTEL_SEC_STATE_PLIMIT)
set_bit(NVDIMM_SECURITY_FROZEN, &security_flags);
if (nd_cmd.cmd.state & ND_INTEL_SEC_STATE_LOCKED)
set_bit(NVDIMM_SECURITY_LOCKED, &security_flags);
else
set_bit(NVDIMM_SECURITY_UNLOCKED, &security_flags);
} else
set_bit(NVDIMM_SECURITY_DISABLED, &security_flags);
return security_flags;
}
static int intel_security_freeze(struct nvdimm *nvdimm)
@ -371,7 +376,7 @@ static void nvdimm_invalidate_cache(void)
#endif
static const struct nvdimm_security_ops __intel_security_ops = {
.state = intel_security_state,
.get_flags = intel_security_flags,
.freeze = intel_security_freeze,
.change_key = intel_security_change_key,
.disable = intel_security_disable,

View File

@ -400,7 +400,7 @@ static int child_unregister(struct device *dev, void *data)
/* We are shutting down. Make state frozen artificially. */
nvdimm_bus_lock(dev);
nvdimm->sec.state = NVDIMM_SECURITY_FROZEN;
set_bit(NVDIMM_SECURITY_FROZEN, &nvdimm->sec.flags);
if (test_and_clear_bit(NDD_WORK_PENDING, &nvdimm->flags))
dev_put = true;
nvdimm_bus_unlock(dev);

View File

@ -372,24 +372,27 @@ __weak ssize_t security_show(struct device *dev,
{
struct nvdimm *nvdimm = to_nvdimm(dev);
switch (nvdimm->sec.state) {
case NVDIMM_SECURITY_DISABLED:
if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
return sprintf(buf, "disabled\n");
case NVDIMM_SECURITY_UNLOCKED:
if (test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.flags))
return sprintf(buf, "unlocked\n");
case NVDIMM_SECURITY_LOCKED:
if (test_bit(NVDIMM_SECURITY_LOCKED, &nvdimm->sec.flags))
return sprintf(buf, "locked\n");
case NVDIMM_SECURITY_FROZEN:
return sprintf(buf, "frozen\n");
case NVDIMM_SECURITY_OVERWRITE:
if (test_bit(NVDIMM_SECURITY_OVERWRITE, &nvdimm->sec.flags))
return sprintf(buf, "overwrite\n");
default:
return -ENOTTY;
}
return -ENOTTY;
}
static ssize_t frozen_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct nvdimm *nvdimm = to_nvdimm(dev);
return sprintf(buf, "%d\n", test_bit(NVDIMM_SECURITY_FROZEN,
&nvdimm->sec.flags));
}
static DEVICE_ATTR_RO(frozen);
#define OPS \
C( OP_FREEZE, "freeze", 1), \
C( OP_DISABLE, "disable", 2), \
@ -501,6 +504,7 @@ static struct attribute *nvdimm_attributes[] = {
&dev_attr_commands.attr,
&dev_attr_available_slots.attr,
&dev_attr_security.attr,
&dev_attr_frozen.attr,
NULL,
};
@ -509,17 +513,24 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
struct device *dev = container_of(kobj, typeof(*dev), kobj);
struct nvdimm *nvdimm = to_nvdimm(dev);
if (a != &dev_attr_security.attr)
if (a != &dev_attr_security.attr && a != &dev_attr_frozen.attr)
return a->mode;
if (nvdimm->sec.state < 0)
if (!nvdimm->sec.flags)
return 0;
/* Are there any state mutation ops? */
if (nvdimm->sec.ops->freeze || nvdimm->sec.ops->disable
|| nvdimm->sec.ops->change_key
|| nvdimm->sec.ops->erase
|| nvdimm->sec.ops->overwrite)
if (a == &dev_attr_security.attr) {
/* Are there any state mutation ops (make writable)? */
if (nvdimm->sec.ops->freeze || nvdimm->sec.ops->disable
|| nvdimm->sec.ops->change_key
|| nvdimm->sec.ops->erase
|| nvdimm->sec.ops->overwrite)
return a->mode;
return 0444;
}
if (nvdimm->sec.ops->freeze)
return a->mode;
return 0444;
return 0;
}
struct attribute_group nvdimm_attribute_group = {
@ -569,8 +580,8 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
* attribute visibility.
*/
/* get security state and extended (master) state */
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.ext_flags = nvdimm_security_flags(nvdimm, NVDIMM_MASTER);
nd_device_register(dev);
return nvdimm;
@ -588,7 +599,7 @@ int nvdimm_security_setup_events(struct device *dev)
{
struct nvdimm *nvdimm = to_nvdimm(dev);
if (nvdimm->sec.state < 0 || !nvdimm->sec.ops
if (!nvdimm->sec.flags || !nvdimm->sec.ops
|| !nvdimm->sec.ops->overwrite)
return 0;
nvdimm->sec.overwrite_state = sysfs_get_dirent(dev->kobj.sd, "security");
@ -614,7 +625,7 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
if (!nvdimm->sec.ops || !nvdimm->sec.ops->freeze)
return -EOPNOTSUPP;
if (nvdimm->sec.state < 0)
if (!nvdimm->sec.flags)
return -EIO;
if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
@ -623,7 +634,7 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
}
rc = nvdimm->sec.ops->freeze(nvdimm);
nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
return rc;
}

View File

@ -39,21 +39,32 @@ struct nvdimm {
const char *dimm_id;
struct {
const struct nvdimm_security_ops *ops;
enum nvdimm_security_state state;
enum nvdimm_security_state ext_state;
unsigned long flags;
unsigned long ext_flags;
unsigned int overwrite_tmo;
struct kernfs_node *overwrite_state;
} sec;
struct delayed_work dwork;
};
static inline enum nvdimm_security_state nvdimm_security_state(
static inline unsigned long nvdimm_security_flags(
struct nvdimm *nvdimm, enum nvdimm_passphrase_type ptype)
{
if (!nvdimm->sec.ops)
return -ENXIO;
u64 flags;
const u64 state_flags = 1UL << NVDIMM_SECURITY_DISABLED
| 1UL << NVDIMM_SECURITY_LOCKED
| 1UL << NVDIMM_SECURITY_UNLOCKED
| 1UL << NVDIMM_SECURITY_OVERWRITE;
return nvdimm->sec.ops->state(nvdimm, ptype);
if (!nvdimm->sec.ops)
return 0;
flags = nvdimm->sec.ops->get_flags(nvdimm, ptype);
/* disabled, locked, unlocked, and overwrite are mutually exclusive */
dev_WARN_ONCE(&nvdimm->dev, hweight64(flags & state_flags) > 1,
"reported invalid security state: %#llx\n",
(unsigned long long) flags);
return flags;
}
int nvdimm_security_freeze(struct nvdimm *nvdimm);
#if IS_ENABLED(CONFIG_NVDIMM_KEYS)

View File

@ -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;
}
rc = check_security_state(nvdimm);
if (rc)
return rc;
if (test_bit(NDD_SECURITY_OVERWRITE, &nvdimm->flags)) {
dev_dbg(dev, "Security operation in progress.\n");
return -EBUSY;
}
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)

View File

@ -160,8 +160,11 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
}
enum nvdimm_security_state {
NVDIMM_SECURITY_ERROR = -1,
/*
* Note that separate bits for locked + unlocked are defined so that
* 'flags == 0' corresponds to an error / not-supported state.
*/
enum nvdimm_security_bits {
NVDIMM_SECURITY_DISABLED,
NVDIMM_SECURITY_UNLOCKED,
NVDIMM_SECURITY_LOCKED,
@ -182,7 +185,7 @@ enum nvdimm_passphrase_type {
};
struct nvdimm_security_ops {
enum nvdimm_security_state (*state)(struct nvdimm *nvdimm,
unsigned long (*get_flags)(struct nvdimm *nvdimm,
enum nvdimm_passphrase_type pass_type);
int (*freeze)(struct nvdimm *nvdimm);
int (*change_key)(struct nvdimm *nvdimm,

View File

@ -18,24 +18,13 @@ ssize_t security_show(struct device *dev,
* For the test version we need to poll the "hardware" in order
* to get the updated status for unlock testing.
*/
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);
switch (nvdimm->sec.state) {
case NVDIMM_SECURITY_DISABLED:
if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
return sprintf(buf, "disabled\n");
case NVDIMM_SECURITY_UNLOCKED:
if (test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.flags))
return sprintf(buf, "unlocked\n");
case NVDIMM_SECURITY_LOCKED:
if (test_bit(NVDIMM_SECURITY_LOCKED, &nvdimm->sec.flags))
return sprintf(buf, "locked\n");
case NVDIMM_SECURITY_FROZEN:
return sprintf(buf, "frozen\n");
case NVDIMM_SECURITY_OVERWRITE:
return sprintf(buf, "overwrite\n");
default:
return -ENOTTY;
}
return -ENOTTY;
}