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>

1 files changed, 32 insertions, 27 deletions

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index cddd0fcf622c..1113b679cd7b 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -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,