nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v13 00/17] Adding security support for nvdimm
@ 2018-12-11 20:25 Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 01/17] acpi/nfit: Add support for Intel DSM 1.8 commands Dave Jiang
                   ` (16 more replies)
  0 siblings, 17 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

The following series implements security support for nvdimm based on Intel
DSM spec v1.8. The passphrase is protected by encrypted-key and managed
through the kernel key management framework. The security features
supported are security state show, passphrase enable/update, passphrase
disable, crypto erase, overwrite, and master passphrase enable/update and
erase. Instead of allowing the security DSMs being issued via ioctl, the
features are managed through a sysfs attribute that accept the relevant
keyid for the encrypted-key(s).

v13:
- Rebased to v4.20-rc5 and combined/squashed various patches from the two
  patch series. Various cleanups from Dan. (Mimi)
- Change encrypted-key nvdimm key format to enc32 key format to make it
  generic for future usages. (Dan)
- Output error code for nvdimm_setup_security_events() failure. (Robert)
- Make nfit_test output consistent. (Robert)

v12:
- Add a mutex for the cached key and remove key_get/key_put messiness (Dan)
- Move security code to its own C file and wrap under CONFIG_NVDIMM_SECURITY
  in order to fix issue reported by 0-day build without CONFIG_KEYS.

v11:
- Dropped keyring usage. (David)
- Fixed up scanf handling. (David)
- Removed callout info for request_key(). (David)
- Included Dan's patches and folded in some changes from Dan. (Dan)
- Made security_show a weak function to allow test override. (Dan)

v10:
- Change usage of strcmp to sysfs_streq. (Dan)
- Lock nvdimm bus when doing secure erase. (Dan)
- Change dev_info to dev_dbg for dimm unlocked success output. (Dan)

v9:
- Addressed various misc comments. (David, Dan)
- Removed init_cred and replaced with current_cred(). (David)
- Changed NVDIMM_PREFIX to char[] constant (David)
- Moved NVDIMM_PREFIX to include/uapi/linux/ndctl.h (Dan)
- Reworked security_update to use old user key to verify against kernel
  key and then update with new user key. (David)
- Added requirement of disable and erase to require old user key for
  verify. (Dan)
- Updated documentation. (Dave)

v8:
- Make the keys retained by the kernel user searchable in order to find the
  key that needs to be updated for key update.

v7:
- Add CONFIG_KEYS depenency for libnvdimm. (Alison)
- Export lookup_user_key(). (David)
- Modified "update" to take two key ids and and use lookup_user_key() in
  order to improve security.  (David)
- Use key ptrs and key_validate() for cached keys. (David)

v6:
- Fix intel DSM data structures to use defined size for passphrase (Robert)
- Fix memcpy size to use sizeof data structure member (Robert)
- Fix defined dimm id length (Robert)
- Making intel_security_ops const (Eric)
- Remove unused var in nvdimm_key_search() (Eric)
- Added wbinvd before secure erase is issued (Robert)
- Removed key_put_sync() usage (David)
- Use init_cred instead of creating own cred (David)
    - Exported init_cred symbol
- Move keyring to dedicated (David)
- Use logon_key_type and friends instead of creating custom (David)
- Use key_lookup() with stored key serial (David)
    - Exported key_lookup() symbol
- Mark passed in key data as const (David)
- Added comment for change_pass_phrase to explain how it works (David)
- Unlink key when it's being removed from keyring. (David)
- Removed request_key() from all security ops except update and unlock.
- Update will now update the existing key's payload with the new key's
  retrieved from userspace when the new payload is accepted by nvdimm.

v5:
- Moved dimm_id initialization (Dan)
- Added a key_put_sync() in order to run key_gc_work and cleanup old key. (Dan)
- Added check to block security state changes while DIMM is active. (Dan)

v4:
- flip payload layout for update passphrase to make it easier on userland.

v3:
- Set x86 wrappers for x86 only bits. (Dan)
- Fixed up some verbiage in commit headers.
- Put in usage of sysfs_streq() for sysfs inputs.
- 0-day build fixes for non-x86 archs.

v2:
- Move inclusion of intel.h to relevant source files and not in nfit.h. (Dan)
- Moved security ring relevant code to dimm_devs.c. (Dan)
- Added dimm_id to nfit_mem to avoid recreate per sysfs show call. (Dan)
- Added routine to return security_ops based on family supplied. (Dan)
- Added nvdimm_key_data struct to wrap raw passphrase string. (Dan)
- Allocate firmware package on stack. (Dan)
- Added missing frozen state detection when retrieving security state.

---

Dan Williams (1):
      acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs

Dave Jiang (16):
      acpi/nfit: Add support for Intel DSM 1.8 commands
      acpi/nfit, libnvdimm: Store dimm id as a member to struct nvdimm
      keys: Export lookup_user_key to external users
      keys-encrypted: add nvdimm key format type to encrypted keys
      acpi/nfit, libnvdimm: Introduce nvdimm_security_ops
      acpi/nfit, libnvdimm: Add freeze security support to Intel nvdimm
      acpi/nfit, libnvdimm: Add disable passphrase support to Intel nvdimm.
      acpi/nfit, libnvdimm: Add enable/update passphrase support for Intel nvdimms
      acpi/nfit, libnvdimm: Add support for issue secure erase DSM to Intel nvdimm
      libnvdimm/security: introduce NDD_SECURITY_BUSY flag
      acpi/nfit, libnvdimm/security: Add security DSM overwrite support
      acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support
      tools/testing/nvdimm: Add test support for Intel nvdimm security DSMs
      tools/testing/nvdimm: Add overwrite support for nfit_test
      tools/testing/nvdimm: add Intel DSM 1.8 support for nfit_test
      libnvdimm/security: Add documentation for nvdimm security support


 Documentation/nvdimm/security.txt                 |  143 +++++++
 Documentation/security/keys/trusted-encrypted.rst |    6 
 drivers/acpi/nfit/Kconfig                         |   11 +
 drivers/acpi/nfit/Makefile                        |    1 
 drivers/acpi/nfit/core.c                          |   93 ++++-
 drivers/acpi/nfit/intel.c                         |  404 ++++++++++++++++++++
 drivers/acpi/nfit/intel.h                         |   76 ++++
 drivers/acpi/nfit/nfit.h                          |   25 +
 drivers/nvdimm/Kconfig                            |    4 
 drivers/nvdimm/Makefile                           |    1 
 drivers/nvdimm/bus.c                              |    8 
 drivers/nvdimm/core.c                             |    3 
 drivers/nvdimm/dimm.c                             |   16 +
 drivers/nvdimm/dimm_devs.c                        |  220 ++++++++++-
 drivers/nvdimm/nd-core.h                          |   45 ++
 drivers/nvdimm/nd.h                               |    3 
 drivers/nvdimm/region_devs.c                      |    7 
 drivers/nvdimm/security.c                         |  431 +++++++++++++++++++++
 include/linux/key.h                               |    3 
 include/linux/libnvdimm.h                         |   68 +++
 security/keys/encrypted-keys/encrypted.c          |   29 +
 security/keys/internal.h                          |    2 
 security/keys/process_keys.c                      |    1 
 tools/testing/nvdimm/Kbuild                       |    3 
 tools/testing/nvdimm/dimm_devs.c                  |   41 ++
 tools/testing/nvdimm/test/nfit.c                  |  321 ++++++++++++++++
 26 files changed, 1923 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/nvdimm/security.txt
 create mode 100644 drivers/acpi/nfit/intel.c
 create mode 100644 drivers/nvdimm/security.c
 create mode 100644 tools/testing/nvdimm/dimm_devs.c

--
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v13 01/17] acpi/nfit: Add support for Intel DSM 1.8 commands
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 02/17] acpi/nfit, libnvdimm: Store dimm id as a member to struct nvdimm Dave Jiang
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add command definition for security commands defined in Intel DSM
specification v1.8 [1]. This includes "get security state", "set
passphrase", "unlock unit", "freeze lock", "secure erase", "overwrite",
"overwrite query", "master passphrase enable/disable", and "master
erase", . Since this adds several Intel definitions, move the relevant
bits to their own header.

These commands mutate physical data, but that manipulation is not cache
coherent. The requirement to flush and invalidate caches makes these
commands unsuitable to be called from userspace, so extra logic is added
to detect and block these commands from being submitted via the ioctl
command submission path.

Lastly, the commands may contain sensitive key material that should not
be dumped in a standard debug session. Update the nvdimm-command
payload-dump facility to move security command payloads behind a
default-off compile time switch.

[1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/Kconfig |   11 +++++++
 drivers/acpi/nfit/core.c  |   44 ++++++++++++++++++++++++---
 drivers/acpi/nfit/intel.h |   74 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/nfit.h  |   21 ++++++++++++-
 drivers/nvdimm/bus.c      |    2 +
 include/linux/libnvdimm.h |    2 +
 6 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/nfit/Kconfig b/drivers/acpi/nfit/Kconfig
index f7c57e33499e..52eefd732cf2 100644
--- a/drivers/acpi/nfit/Kconfig
+++ b/drivers/acpi/nfit/Kconfig
@@ -13,3 +13,14 @@ config ACPI_NFIT
 
 	  To compile this driver as a module, choose M here:
 	  the module will be called nfit.
+
+config NFIT_SECURITY_DEBUG
+	bool "Enable debug for NVDIMM security commands"
+	depends on ACPI_NFIT
+	help
+	  Some NVDIMM devices and controllers support encryption and
+	  other security features. The payloads for the commands that
+	  enable those features may contain sensitive clear-text
+	  security material. Disable debug of those command payloads
+	  by default. If you are a kernel developer actively working
+	  on NVDIMM security enabling say Y, otherwise say N.
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 14d9f5bea015..68d146f8f8a6 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -24,6 +24,7 @@
 #include <linux/nd.h>
 #include <asm/cacheflush.h>
 #include <acpi/nfit.h>
+#include "intel.h"
 #include "nfit.h"
 #include "intel.h"
 
@@ -380,6 +381,14 @@ static u8 nfit_dsm_revid(unsigned family, unsigned func)
 			[NVDIMM_INTEL_QUERY_FWUPDATE] = 2,
 			[NVDIMM_INTEL_SET_THRESHOLD] = 2,
 			[NVDIMM_INTEL_INJECT_ERROR] = 2,
+			[NVDIMM_INTEL_GET_SECURITY_STATE] = 2,
+			[NVDIMM_INTEL_SET_PASSPHRASE] = 2,
+			[NVDIMM_INTEL_DISABLE_PASSPHRASE] = 2,
+			[NVDIMM_INTEL_UNLOCK_UNIT] = 2,
+			[NVDIMM_INTEL_FREEZE_LOCK] = 2,
+			[NVDIMM_INTEL_SECURE_ERASE] = 2,
+			[NVDIMM_INTEL_OVERWRITE] = 2,
+			[NVDIMM_INTEL_QUERY_OVERWRITE] = 2,
 		},
 	};
 	u8 id;
@@ -394,6 +403,15 @@ static u8 nfit_dsm_revid(unsigned family, unsigned func)
 	return id;
 }
 
+static bool payload_dumpable(unsigned int family, unsigned int func)
+{
+	if (family == NVDIMM_FAMILY_INTEL
+			&& func >= NVDIMM_INTEL_GET_SECURITY_STATE
+			&& func <= NVDIMM_INTEL_MASTER_SECURE_ERASE)
+		return IS_ENABLED(CONFIG_NFIT_SECURITY_DEBUG);
+	return true;
+}
+
 int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
 {
@@ -478,9 +496,10 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 
 	dev_dbg(dev, "%s cmd: %d: func: %d input length: %d\n",
 		dimm_name, cmd, func, in_buf.buffer.length);
-	print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
-			in_buf.buffer.pointer,
-			min_t(u32, 256, in_buf.buffer.length), true);
+	if (payload_dumpable(nfit_mem->family, func))
+		print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
+				in_buf.buffer.pointer,
+				min_t(u32, 256, in_buf.buffer.length), true);
 
 	/* call the BIOS, prefer the named methods over _DSM if available */
 	if (nvdimm && cmd == ND_CMD_GET_CONFIG_SIZE
@@ -3337,7 +3356,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
 	return 0;
 }
 
-static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
+static int __acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
 		struct nvdimm *nvdimm, unsigned int cmd)
 {
 	struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc);
@@ -3359,6 +3378,23 @@ static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
 	return 0;
 }
 
+/* prevent security commands from being issued via ioctl */
+static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
+		struct nvdimm *nvdimm, unsigned int cmd, void *buf)
+{
+	struct nd_cmd_pkg *call_pkg = buf;
+	unsigned int func;
+
+	if (nvdimm && cmd == ND_CMD_CALL &&
+			call_pkg->nd_family == NVDIMM_FAMILY_INTEL) {
+		func = call_pkg->nd_command;
+		if ((1 << func) & NVDIMM_INTEL_SECURITY_CMDMASK)
+			return -EOPNOTSUPP;
+	}
+
+	return __acpi_nfit_clear_to_send(nd_desc, nvdimm, cmd);
+}
+
 int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc,
 		enum nfit_ars_state req_type)
 {
diff --git a/drivers/acpi/nfit/intel.h b/drivers/acpi/nfit/intel.h
index 86746312381f..1802bd398c23 100644
--- a/drivers/acpi/nfit/intel.h
+++ b/drivers/acpi/nfit/intel.h
@@ -35,4 +35,78 @@ struct nd_intel_smart {
 	};
 } __packed;
 
+#define ND_INTEL_STATUS_SIZE		4
+#define ND_INTEL_PASSPHRASE_SIZE	32
+
+#define ND_INTEL_STATUS_NOT_SUPPORTED	1
+#define ND_INTEL_STATUS_RETRY		5
+#define ND_INTEL_STATUS_NOT_READY	9
+#define ND_INTEL_STATUS_INVALID_STATE	10
+#define ND_INTEL_STATUS_INVALID_PASS	11
+#define ND_INTEL_STATUS_OVERWRITE_UNSUPPORTED	0x10007
+#define ND_INTEL_STATUS_OQUERY_INPROGRESS	0x10007
+#define ND_INTEL_STATUS_OQUERY_SEQUENCE_ERR	0x20007
+
+#define ND_INTEL_SEC_STATE_ENABLED	0x02
+#define ND_INTEL_SEC_STATE_LOCKED	0x04
+#define ND_INTEL_SEC_STATE_FROZEN	0x08
+#define ND_INTEL_SEC_STATE_PLIMIT	0x10
+#define ND_INTEL_SEC_STATE_UNSUPPORTED	0x20
+#define ND_INTEL_SEC_STATE_OVERWRITE	0x40
+
+#define ND_INTEL_SEC_ESTATE_ENABLED	0x01
+#define ND_INTEL_SEC_ESTATE_PLIMIT	0x02
+
+struct nd_intel_get_security_state {
+	u32 status;
+	u8 extended_state;
+	u8 reserved[3];
+	u8 state;
+	u8 reserved1[3];
+} __packed;
+
+struct nd_intel_set_passphrase {
+	u8 old_pass[ND_INTEL_PASSPHRASE_SIZE];
+	u8 new_pass[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_unlock_unit {
+	u8 passphrase[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_disable_passphrase {
+	u8 passphrase[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_freeze_lock {
+	u32 status;
+} __packed;
+
+struct nd_intel_secure_erase {
+	u8 passphrase[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_overwrite {
+	u8 passphrase[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_query_overwrite {
+	u32 status;
+} __packed;
+
+struct nd_intel_set_master_passphrase {
+	u8 old_pass[ND_INTEL_PASSPHRASE_SIZE];
+	u8 new_pass[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
+
+struct nd_intel_master_secure_erase {
+	u8 passphrase[ND_INTEL_PASSPHRASE_SIZE];
+	u32 status;
+} __packed;
 #endif
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index df0f6b8407e7..ecde13a9199d 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -60,14 +60,33 @@ enum nvdimm_family_cmds {
 	NVDIMM_INTEL_QUERY_FWUPDATE = 16,
 	NVDIMM_INTEL_SET_THRESHOLD = 17,
 	NVDIMM_INTEL_INJECT_ERROR = 18,
+	NVDIMM_INTEL_GET_SECURITY_STATE = 19,
+	NVDIMM_INTEL_SET_PASSPHRASE = 20,
+	NVDIMM_INTEL_DISABLE_PASSPHRASE = 21,
+	NVDIMM_INTEL_UNLOCK_UNIT = 22,
+	NVDIMM_INTEL_FREEZE_LOCK = 23,
+	NVDIMM_INTEL_SECURE_ERASE = 24,
+	NVDIMM_INTEL_OVERWRITE = 25,
+	NVDIMM_INTEL_QUERY_OVERWRITE = 26,
+	NVDIMM_INTEL_SET_MASTER_PASSPHRASE = 27,
+	NVDIMM_INTEL_MASTER_SECURE_ERASE = 28,
 };
 
+#define NVDIMM_INTEL_SECURITY_CMDMASK \
+(1 << NVDIMM_INTEL_GET_SECURITY_STATE | 1 << NVDIMM_INTEL_SET_PASSPHRASE \
+| 1 << NVDIMM_INTEL_DISABLE_PASSPHRASE | 1 << NVDIMM_INTEL_UNLOCK_UNIT \
+| 1 << NVDIMM_INTEL_FREEZE_LOCK | 1 << NVDIMM_INTEL_SECURE_ERASE \
+| 1 << NVDIMM_INTEL_OVERWRITE | 1 << NVDIMM_INTEL_QUERY_OVERWRITE \
+| 1 << NVDIMM_INTEL_SET_MASTER_PASSPHRASE \
+| 1 << NVDIMM_INTEL_MASTER_SECURE_ERASE)
+
 #define NVDIMM_INTEL_CMDMASK \
 (NVDIMM_STANDARD_CMDMASK | 1 << NVDIMM_INTEL_GET_MODES \
  | 1 << NVDIMM_INTEL_GET_FWINFO | 1 << NVDIMM_INTEL_START_FWUPDATE \
  | 1 << NVDIMM_INTEL_SEND_FWUPDATE | 1 << NVDIMM_INTEL_FINISH_FWUPDATE \
  | 1 << NVDIMM_INTEL_QUERY_FWUPDATE | 1 << NVDIMM_INTEL_SET_THRESHOLD \
- | 1 << NVDIMM_INTEL_INJECT_ERROR | 1 << NVDIMM_INTEL_LATCH_SHUTDOWN)
+ | 1 << NVDIMM_INTEL_INJECT_ERROR | 1 << NVDIMM_INTEL_LATCH_SHUTDOWN \
+ | NVDIMM_INTEL_SECURITY_CMDMASK)
 
 enum nfit_uuids {
 	/* for simplicity alias the uuid index with the family id */
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index f1fb39921236..9743d8083538 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -902,7 +902,7 @@ static int nd_cmd_clear_to_send(struct nvdimm_bus *nvdimm_bus,
 
 	/* ask the bus provider if it would like to block this request */
 	if (nd_desc->clear_to_send) {
-		int rc = nd_desc->clear_to_send(nd_desc, nvdimm, cmd);
+		int rc = nd_desc->clear_to_send(nd_desc, nvdimm, cmd, data);
 
 		if (rc)
 			return rc;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 097072c5a852..472171af7f60 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -87,7 +87,7 @@ struct nvdimm_bus_descriptor {
 	ndctl_fn ndctl;
 	int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);
 	int (*clear_to_send)(struct nvdimm_bus_descriptor *nd_desc,
-			struct nvdimm *nvdimm, unsigned int cmd);
+			struct nvdimm *nvdimm, unsigned int cmd, void *data);
 };
 
 struct nd_cmd_desc {

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 02/17] acpi/nfit, libnvdimm: Store dimm id as a member to struct nvdimm
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 01/17] acpi/nfit: Add support for Intel DSM 1.8 commands Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 03/17] keys: Export lookup_user_key to external users Dave Jiang
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

The generated dimm id is needed for the sysfs attribute as well as being
used as the identifier/description for the security key. Since it's
constant and should never change, store it as a member of struct nvdimm.

As nvdimm_create() continues to grow parameters relative to NFIT driver
requirements, do not require other implementations to keep pace.
Introduce __nvdimm_create() to carry the new parameters and keep
nvdimm_create() with the long standing default api.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/core.c   |   31 ++++++++++++++++++-------------
 drivers/acpi/nfit/nfit.h   |    3 +++
 drivers/nvdimm/dimm_devs.c |   12 +++++++-----
 drivers/nvdimm/nd-core.h   |    1 +
 include/linux/libnvdimm.h  |   17 +++++++++++++----
 5 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 68d146f8f8a6..c74f087398fb 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1592,18 +1592,10 @@ static DEVICE_ATTR_RO(flags);
 static ssize_t id_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
 
-	if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
-		return sprintf(buf, "%04x-%02x-%04x-%08x\n",
-				be16_to_cpu(dcr->vendor_id),
-				dcr->manufacturing_location,
-				be16_to_cpu(dcr->manufacturing_date),
-				be32_to_cpu(dcr->serial_number));
-	else
-		return sprintf(buf, "%04x-%08x\n",
-				be16_to_cpu(dcr->vendor_id),
-				be32_to_cpu(dcr->serial_number));
+	return sprintf(buf, "%s\n", nfit_mem->id);
 }
 static DEVICE_ATTR_RO(id);
 
@@ -1799,10 +1791,23 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 	const guid_t *guid;
 	int i;
 	int family = -1;
+	struct acpi_nfit_control_region *dcr = nfit_mem->dcr;
 
 	/* nfit test assumes 1:1 relationship between commands and dsms */
 	nfit_mem->dsm_mask = acpi_desc->dimm_cmd_force_en;
 	nfit_mem->family = NVDIMM_FAMILY_INTEL;
+
+	if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
+		sprintf(nfit_mem->id, "%04x-%02x-%04x-%08x",
+				be16_to_cpu(dcr->vendor_id),
+				dcr->manufacturing_location,
+				be16_to_cpu(dcr->manufacturing_date),
+				be32_to_cpu(dcr->serial_number));
+	else
+		sprintf(nfit_mem->id, "%04x-%08x",
+				be16_to_cpu(dcr->vendor_id),
+				be32_to_cpu(dcr->serial_number));
+
 	adev = to_acpi_dev(acpi_desc);
 	if (!adev) {
 		/* unit test case */
@@ -1989,10 +1994,10 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 
 		flush = nfit_mem->nfit_flush ? nfit_mem->nfit_flush->flush
 			: NULL;
-		nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
+		nvdimm = __nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
 				acpi_nfit_dimm_attribute_groups,
 				flags, cmd_mask, flush ? flush->hint_count : 0,
-				nfit_mem->flush_wpq);
+				nfit_mem->flush_wpq, &nfit_mem->id[0]);
 		if (!nvdimm)
 			return -ENOMEM;
 
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index ecde13a9199d..33691aecfcee 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -183,6 +183,8 @@ enum nfit_mem_flags {
 	NFIT_MEM_DIRTY_COUNT,
 };
 
+#define NFIT_DIMM_ID_LEN	22
+
 /* assembled tables for a given dimm/memory-device */
 struct nfit_mem {
 	struct nvdimm *nvdimm;
@@ -200,6 +202,7 @@ struct nfit_mem {
 	struct list_head list;
 	struct acpi_device *adev;
 	struct acpi_nfit_desc *acpi_desc;
+	char id[NFIT_DIMM_ID_LEN+1];
 	struct resource *flush_wpq;
 	unsigned long dsm_mask;
 	unsigned long flags;
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 6c3de2317390..508dd405f84f 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -383,10 +383,10 @@ struct attribute_group nvdimm_attribute_group = {
 };
 EXPORT_SYMBOL_GPL(nvdimm_attribute_group);
 
-struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
-		const struct attribute_group **groups, unsigned long flags,
-		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq)
+struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
+		void *provider_data, const struct attribute_group **groups,
+		unsigned long flags, unsigned long cmd_mask, int num_flush,
+		struct resource *flush_wpq, const char *dimm_id)
 {
 	struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
 	struct device *dev;
@@ -399,6 +399,8 @@ struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		kfree(nvdimm);
 		return NULL;
 	}
+
+	nvdimm->dimm_id = dimm_id;
 	nvdimm->provider_data = provider_data;
 	nvdimm->flags = flags;
 	nvdimm->cmd_mask = cmd_mask;
@@ -415,7 +417,7 @@ struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 
 	return nvdimm;
 }
-EXPORT_SYMBOL_GPL(nvdimm_create);
+EXPORT_SYMBOL_GPL(__nvdimm_create);
 
 int alias_dpa_busy(struct device *dev, void *data)
 {
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 182258f64417..ff26876e6ea3 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -41,6 +41,7 @@ struct nvdimm {
 	atomic_t busy;
 	int id, num_flush;
 	struct resource *flush_wpq;
+	const char *dimm_id;
 };
 
 /**
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 472171af7f60..f980046b9588 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -175,10 +175,19 @@ const char *nvdimm_name(struct nvdimm *nvdimm);
 struct kobject *nvdimm_kobj(struct nvdimm *nvdimm);
 unsigned long nvdimm_cmd_mask(struct nvdimm *nvdimm);
 void *nvdimm_provider_data(struct nvdimm *nvdimm);
-struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
-		const struct attribute_group **groups, unsigned long flags,
-		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq);
+struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
+		void *provider_data, const struct attribute_group **groups,
+		unsigned long flags, unsigned long cmd_mask, int num_flush,
+		struct resource *flush_wpq, const char *dimm_id);
+static inline struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus,
+		void *provider_data, const struct attribute_group **groups,
+		unsigned long flags, unsigned long cmd_mask, int num_flush,
+		struct resource *flush_wpq)
+{
+	return __nvdimm_create(nvdimm_bus, provider_data, groups, flags,
+			cmd_mask, num_flush, flush_wpq, NULL);
+}
+
 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,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 03/17] keys: Export lookup_user_key to external users
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 01/17] acpi/nfit: Add support for Intel DSM 1.8 commands Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 02/17] acpi/nfit, libnvdimm: Store dimm id as a member to struct nvdimm Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys Dave Jiang
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Export lookup_user_key() symbol in order to allow nvdimm passphrase
update to retrieve user injected keys.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/key.h          |    3 +++
 security/keys/internal.h     |    2 --
 security/keys/process_keys.c |    1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index e58ee10f6e58..7099985e35a9 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -346,6 +346,9 @@ static inline key_serial_t key_serial(const struct key *key)
 
 extern void key_set_timeout(struct key *, unsigned);
 
+extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
+				 key_perm_t perm);
+
 /*
  * The permissions required on a key that we're looking up.
  */
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 74cb0ff42fed..479909b858c7 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -158,8 +158,6 @@ extern struct key *request_key_and_link(struct key_type *type,
 
 extern bool lookup_user_key_possessed(const struct key *key,
 				      const struct key_match_data *match_data);
-extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
-				 key_perm_t perm);
 #define KEY_LOOKUP_CREATE	0x01
 #define KEY_LOOKUP_PARTIAL	0x02
 #define KEY_LOOKUP_FOR_UNLINK	0x04
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index d5b25e535d3a..ec4fd4531224 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -755,6 +755,7 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 	put_cred(ctx.cred);
 	goto try_again;
 }
+EXPORT_SYMBOL(lookup_user_key);
 
 /*
  * Join the named keyring as the session keyring if possible else attempt to

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (2 preceding siblings ...)
  2018-12-11 20:25 ` [PATCH v13 03/17] keys: Export lookup_user_key to external users Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-12 10:51   ` Mimi Zohar
  2018-12-11 20:25 ` [PATCH v13 05/17] acpi/nfit, libnvdimm: Introduce nvdimm_security_ops Dave Jiang
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Adding nvdimm key format type to encrypted keys in order to limit the size
of the key to 32bytes.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/security/keys/trusted-encrypted.rst |    6 ++++
 security/keys/encrypted-keys/encrypted.c          |   29 ++++++++++++++-------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 3bb24e09a332..e8a1c35cd277 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -76,7 +76,7 @@ Usage::
 
 Where::
 
-	format:= 'default | ecryptfs'
+	format:= 'default | ecryptfs | enc32'
 	key-type:= 'trusted' | 'user'
 
 
@@ -173,3 +173,7 @@ are anticipated.  In particular the new format 'ecryptfs' has been defined in
 in order to use encrypted keys to mount an eCryptfs filesystem.  More details
 about the usage can be found in the file
 ``Documentation/security/keys/ecryptfs.rst``.
+
+Another new format 'enc32' has been defined in order to support encrypted keys
+with payload size of 32 bytes. This will initially be used for nvdimm security
+but may expand to other usages that require 32 bytes payload.
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index d92cbf9687c3..fe0aefd06f83 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -45,6 +45,7 @@ static const char hmac_alg[] = "hmac(sha256)";
 static const char blkcipher_alg[] = "cbc(aes)";
 static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
+static const char key_format_enc32[] = "enc32";
 static unsigned int ivsize;
 static int blksize;
 
@@ -54,6 +55,7 @@ static int blksize;
 #define HASH_SIZE SHA256_DIGEST_SIZE
 #define MAX_DATA_SIZE 4096
 #define MIN_DATA_SIZE  20
+#define KEY_ENC32_PAYLOAD_LEN 32
 
 static struct crypto_shash *hash_tfm;
 
@@ -62,12 +64,13 @@ enum {
 };
 
 enum {
-	Opt_error = -1, Opt_default, Opt_ecryptfs
+	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_enc32
 };
 
 static const match_table_t key_format_tokens = {
 	{Opt_default, "default"},
 	{Opt_ecryptfs, "ecryptfs"},
+	{Opt_enc32, "enc32"},
 	{Opt_error, NULL}
 };
 
@@ -195,6 +198,7 @@ static int datablob_parse(char *datablob, const char **format,
 	key_format = match_token(p, key_format_tokens, args);
 	switch (key_format) {
 	case Opt_ecryptfs:
+	case Opt_enc32:
 	case Opt_default:
 		*format = p;
 		*master_desc = strsep(&datablob, " \t");
@@ -625,15 +629,22 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
 	format_len = (!format) ? strlen(key_format_default) : strlen(format);
 	decrypted_datalen = dlen;
 	payload_datalen = decrypted_datalen;
-	if (format && !strcmp(format, key_format_ecryptfs)) {
-		if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
-			pr_err("encrypted_key: keylen for the ecryptfs format "
-			       "must be equal to %d bytes\n",
-			       ECRYPTFS_MAX_KEY_BYTES);
-			return ERR_PTR(-EINVAL);
+	if (format) {
+		if (!strcmp(format, key_format_ecryptfs)) {
+			if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
+				pr_err("encrypted_key: keylen for the ecryptfs format must be equal to %d bytes\n",
+					ECRYPTFS_MAX_KEY_BYTES);
+				return ERR_PTR(-EINVAL);
+			}
+			decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
+			payload_datalen = sizeof(struct ecryptfs_auth_tok);
+		} else if (!strcmp(format, key_format_enc32)) {
+			if (decrypted_datalen != KEY_ENC32_PAYLOAD_LEN) {
+				pr_err("encrypted_key: enc32 key payload incorrect length: %d\n",
+						decrypted_datalen);
+				return ERR_PTR(-EINVAL);
+			}
 		}
-		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
-		payload_datalen = sizeof(struct ecryptfs_auth_tok);
 	}
 
 	encrypted_datalen = roundup(decrypted_datalen, blksize);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 05/17] acpi/nfit, libnvdimm: Introduce nvdimm_security_ops
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (3 preceding siblings ...)
  2018-12-11 20:25 ` [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-11 20:25 ` [PATCH v13 06/17] acpi/nfit, libnvdimm: Add freeze security support to Intel nvdimm Dave Jiang
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Some NVDIMMs, like the ones defined by the NVDIMM_FAMILY_INTEL command
set, expose a security capability to lock the DIMMs at poweroff and
require a passphrase to unlock them. The security model is derived from
ATA security. In anticipation of other DIMMs implementing a similar
scheme, and to abstract the core security implementation away from the
device-specific details, introduce nvdimm_security_ops.

Initially only a status retrieval operation, ->state(), is defined,
along with the base infrastructure and definitions for future
operations.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/Makefile  |    1 +
 drivers/acpi/nfit/core.c    |   13 ++++++++++
 drivers/acpi/nfit/intel.c   |   54 +++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/intel.h   |    2 ++
 drivers/nvdimm/bus.c        |    6 +++++
 drivers/nvdimm/dimm_devs.c  |   45 +++++++++++++++++++++++++++++++++++-
 drivers/nvdimm/nd-core.h    |   13 ++++++++++
 include/linux/libnvdimm.h   |   27 ++++++++++++++++++++--
 tools/testing/nvdimm/Kbuild |    1 +
 9 files changed, 158 insertions(+), 4 deletions(-)
 create mode 100644 drivers/acpi/nfit/intel.c

diff --git a/drivers/acpi/nfit/Makefile b/drivers/acpi/nfit/Makefile
index a407e769f103..751081c47886 100644
--- a/drivers/acpi/nfit/Makefile
+++ b/drivers/acpi/nfit/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_ACPI_NFIT) := nfit.o
 nfit-y := core.o
+nfit-y += intel.o
 nfit-$(CONFIG_X86_MCE) += mce.o
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index c74f087398fb..77f188cd8023 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1928,6 +1928,16 @@ static void shutdown_dimm_notify(void *data)
 	mutex_unlock(&acpi_desc->init_mutex);
 }
 
+static const struct nvdimm_security_ops *acpi_nfit_get_security_ops(int family)
+{
+	switch (family) {
+	case NVDIMM_FAMILY_INTEL:
+		return intel_security_ops;
+	default:
+		return NULL;
+	}
+}
+
 static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 {
 	struct nfit_mem *nfit_mem;
@@ -1997,7 +2007,8 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 		nvdimm = __nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
 				acpi_nfit_dimm_attribute_groups,
 				flags, cmd_mask, flush ? flush->hint_count : 0,
-				nfit_mem->flush_wpq, &nfit_mem->id[0]);
+				nfit_mem->flush_wpq, &nfit_mem->id[0],
+				acpi_nfit_get_security_ops(nfit_mem->family));
 		if (!nvdimm)
 			return -ENOMEM;
 
diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
new file mode 100644
index 000000000000..fd7a8f6d2c20
--- /dev/null
+++ b/drivers/acpi/nfit/intel.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+#include <linux/libnvdimm.h>
+#include <linux/ndctl.h>
+#include <linux/acpi.h>
+#include "intel.h"
+#include "nfit.h"
+
+static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_get_security_state cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_GET_SECURITY_STATE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_out =
+				sizeof(struct nd_intel_get_security_state),
+			.nd_fw_size =
+				sizeof(struct nd_intel_get_security_state),
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_GET_SECURITY_STATE, &nfit_mem->dsm_mask))
+		return -ENXIO;
+
+	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;
+
+	/* check and see if security is enabled and locked */
+	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;
+	}
+	return NVDIMM_SECURITY_DISABLED;
+}
+
+static const struct nvdimm_security_ops __intel_security_ops = {
+	.state = intel_security_state,
+};
+const struct nvdimm_security_ops *intel_security_ops = &__intel_security_ops;
diff --git a/drivers/acpi/nfit/intel.h b/drivers/acpi/nfit/intel.h
index 1802bd398c23..0aca682ab9d7 100644
--- a/drivers/acpi/nfit/intel.h
+++ b/drivers/acpi/nfit/intel.h
@@ -35,6 +35,8 @@ struct nd_intel_smart {
 	};
 } __packed;
 
+extern const struct nvdimm_security_ops *intel_security_ops;
+
 #define ND_INTEL_STATUS_SIZE		4
 #define ND_INTEL_PASSPHRASE_SIZE	32
 
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 9743d8083538..eae17d8ee539 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -331,6 +331,12 @@ struct nvdimm_bus *to_nvdimm_bus(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(to_nvdimm_bus);
 
+struct nvdimm_bus *nvdimm_to_bus(struct nvdimm *nvdimm)
+{
+	return to_nvdimm_bus(nvdimm->dev.parent);
+}
+EXPORT_SYMBOL_GPL(nvdimm_to_bus);
+
 struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
 		struct nvdimm_bus_descriptor *nd_desc)
 {
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 508dd405f84f..9609b671311b 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -370,23 +370,60 @@ static ssize_t available_slots_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(available_slots);
 
+static ssize_t security_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+
+	switch (nvdimm->sec.state) {
+	case NVDIMM_SECURITY_DISABLED:
+		return sprintf(buf, "disabled\n");
+	case NVDIMM_SECURITY_UNLOCKED:
+		return sprintf(buf, "unlocked\n");
+	case NVDIMM_SECURITY_LOCKED:
+		return sprintf(buf, "locked\n");
+	case NVDIMM_SECURITY_FROZEN:
+		return sprintf(buf, "frozen\n");
+	case NVDIMM_SECURITY_OVERWRITE:
+		return sprintf(buf, "overwrite\n");
+	}
+
+	return -ENOTTY;
+}
+static DEVICE_ATTR_RO(security);
+
 static struct attribute *nvdimm_attributes[] = {
 	&dev_attr_state.attr,
 	&dev_attr_flags.attr,
 	&dev_attr_commands.attr,
 	&dev_attr_available_slots.attr,
+	&dev_attr_security.attr,
 	NULL,
 };
 
+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)
+		return a->mode;
+	if (nvdimm->sec.state < 0)
+		return 0;
+	return a->mode;
+}
+
 struct attribute_group nvdimm_attribute_group = {
 	.attrs = nvdimm_attributes,
+	.is_visible = nvdimm_visible,
 };
 EXPORT_SYMBOL_GPL(nvdimm_attribute_group);
 
 struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 		void *provider_data, const struct attribute_group **groups,
 		unsigned long flags, unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq, const char *dimm_id)
+		struct resource *flush_wpq, const char *dimm_id,
+		const struct nvdimm_security_ops *sec_ops)
 {
 	struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
 	struct device *dev;
@@ -413,6 +450,12 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 	dev->type = &nvdimm_device_type;
 	dev->devt = MKDEV(nvdimm_major, nvdimm->id);
 	dev->groups = groups;
+	nvdimm->sec.ops = sec_ops;
+	/*
+	 * Security state must be initialized before device_add() for
+	 * attribute visibility.
+	 */
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
 	nd_device_register(dev);
 
 	return nvdimm;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index ff26876e6ea3..1919f5c0d581 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -42,8 +42,21 @@ struct nvdimm {
 	int id, num_flush;
 	struct resource *flush_wpq;
 	const char *dimm_id;
+	struct {
+		const struct nvdimm_security_ops *ops;
+		enum nvdimm_security_state state;
+	} sec;
 };
 
+static inline enum nvdimm_security_state nvdimm_security_state(
+		struct nvdimm *nvdimm)
+{
+	if (!nvdimm->sec.ops)
+		return -ENXIO;
+
+	return nvdimm->sec.ops->state(nvdimm);
+}
+
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
  * @nd_mapping: blk region mapping boundaries
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index f980046b9588..f4d63f49f7dd 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -155,6 +155,18 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
 
 }
 
+enum nvdimm_security_state {
+	NVDIMM_SECURITY_DISABLED,
+	NVDIMM_SECURITY_UNLOCKED,
+	NVDIMM_SECURITY_LOCKED,
+	NVDIMM_SECURITY_FROZEN,
+	NVDIMM_SECURITY_OVERWRITE,
+};
+
+struct nvdimm_security_ops {
+	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
+};
+
 void badrange_init(struct badrange *badrange);
 int badrange_add(struct badrange *badrange, u64 addr, u64 length);
 void badrange_forget(struct badrange *badrange, phys_addr_t start,
@@ -165,6 +177,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
 		struct nvdimm_bus_descriptor *nfit_desc);
 void nvdimm_bus_unregister(struct nvdimm_bus *nvdimm_bus);
 struct nvdimm_bus *to_nvdimm_bus(struct device *dev);
+struct nvdimm_bus *nvdimm_to_bus(struct nvdimm *nvdimm);
 struct nvdimm *to_nvdimm(struct device *dev);
 struct nd_region *to_nd_region(struct device *dev);
 struct device *nd_region_dev(struct nd_region *nd_region);
@@ -178,14 +191,15 @@ void *nvdimm_provider_data(struct nvdimm *nvdimm);
 struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 		void *provider_data, const struct attribute_group **groups,
 		unsigned long flags, unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq, const char *dimm_id);
+		struct resource *flush_wpq, const char *dimm_id,
+		const struct nvdimm_security_ops *sec_ops);
 static inline struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 		void *provider_data, const struct attribute_group **groups,
 		unsigned long flags, unsigned long cmd_mask, int num_flush,
 		struct resource *flush_wpq)
 {
 	return __nvdimm_create(nvdimm_bus, provider_data, groups, flags,
-			cmd_mask, num_flush, flush_wpq, NULL);
+			cmd_mask, num_flush, flush_wpq, NULL, NULL);
 }
 
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd);
@@ -214,6 +228,15 @@ void nvdimm_flush(struct nd_region *nd_region);
 int nvdimm_has_flush(struct nd_region *nd_region);
 int nvdimm_has_cache(struct nd_region *nd_region);
 
+static inline int nvdimm_ctl(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
+		unsigned int buf_len, int *cmd_rc)
+{
+	struct nvdimm_bus *nvdimm_bus = nvdimm_to_bus(nvdimm);
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+
+	return nd_desc->ndctl(nd_desc, nvdimm, cmd, buf, buf_len, cmd_rc);
+}
+
 #ifdef CONFIG_ARCH_HAS_PMEM_API
 #define ARCH_MEMREMAP_PMEM MEMREMAP_WB
 void arch_wb_cache_pmem(void *addr, size_t size);
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 778ceb651000..4a2f3cff2a75 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -37,6 +37,7 @@ obj-$(CONFIG_DEV_DAX) += device_dax.o
 obj-$(CONFIG_DEV_DAX_PMEM) += dax_pmem.o
 
 nfit-y := $(ACPI_SRC)/core.o
+nfit-y += $(ACPI_SRC)/intel.o
 nfit-$(CONFIG_X86_MCE) += $(ACPI_SRC)/mce.o
 nfit-y += acpi_nfit_test.o
 nfit-y += config_check.o

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 06/17] acpi/nfit, libnvdimm: Add freeze security support to Intel nvdimm
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (4 preceding siblings ...)
  2018-12-11 20:25 ` [PATCH v13 05/17] acpi/nfit, libnvdimm: Introduce nvdimm_security_ops Dave Jiang
@ 2018-12-11 20:25 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 07/17] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs Dave Jiang
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:25 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add support for freeze security on Intel nvdimm. This locks out any
changes to security for the DIMM until a hard reset of the DIMM is
performed. This is triggered by writing "freeze" to the generic
nvdimm/nmemX "security" sysfs attribute.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/intel.c  |   28 +++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   66 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/nvdimm/nd-core.h   |    1 +
 include/linux/libnvdimm.h  |    1 +
 4 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index fd7a8f6d2c20..f98d680d1a39 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -48,7 +48,35 @@ static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
 	return NVDIMM_SECURITY_DISABLED;
 }
 
+static int intel_security_freeze(struct nvdimm *nvdimm)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_freeze_lock cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_FREEZE_LOCK,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_FREEZE_LOCK, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	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;
+	return 0;
+}
+
 static const struct nvdimm_security_ops __intel_security_ops = {
 	.state = intel_security_state,
+	.freeze = intel_security_freeze,
 };
 const struct nvdimm_security_ops *intel_security_ops = &__intel_security_ops;
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 9609b671311b..8e0bd2ce4dd0 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -390,7 +390,48 @@ static ssize_t security_show(struct device *dev,
 
 	return -ENOTTY;
 }
-static DEVICE_ATTR_RO(security);
+
+static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	ssize_t rc;
+
+	if (atomic_read(&nvdimm->busy))
+		return -EBUSY;
+
+	if (sysfs_streq(buf, "freeze")) {
+		dev_dbg(dev, "freeze\n");
+		rc = nvdimm_security_freeze(nvdimm);
+	} else
+		return -EINVAL;
+
+	if (rc == 0)
+		rc = len;
+	return rc;
+
+}
+
+static ssize_t security_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+
+{
+	ssize_t rc;
+
+	/*
+	 * Require all userspace triggered security management to be
+	 * done while probing is idle and the DIMM is not in active use
+	 * in any region.
+	 */
+	device_lock(dev);
+	nvdimm_bus_lock(dev);
+	wait_nvdimm_bus_probe_idle(dev);
+	rc = __security_store(dev, buf, len);
+	nvdimm_bus_unlock(dev);
+	device_unlock(dev);
+
+	return rc;
+}
+static DEVICE_ATTR_RW(security);
 
 static struct attribute *nvdimm_attributes[] = {
 	&dev_attr_state.attr,
@@ -410,7 +451,10 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
 		return a->mode;
 	if (nvdimm->sec.state < 0)
 		return 0;
-	return a->mode;
+	/* Are there any state mutation ops? */
+	if (nvdimm->sec.ops->freeze)
+		return a->mode;
+	return 0444;
 }
 
 struct attribute_group nvdimm_attribute_group = {
@@ -462,6 +506,24 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 }
 EXPORT_SYMBOL_GPL(__nvdimm_create);
 
+int nvdimm_security_freeze(struct nvdimm *nvdimm)
+{
+	int rc;
+
+	WARN_ON_ONCE(!is_nvdimm_bus_locked(&nvdimm->dev));
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->freeze)
+		return -EOPNOTSUPP;
+
+	if (nvdimm->sec.state < 0)
+		return -EIO;
+
+	rc = nvdimm->sec.ops->freeze(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+
+	return rc;
+}
+
 int alias_dpa_busy(struct device *dev, void *data)
 {
 	resource_size_t map_end, blk_start, new;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 1919f5c0d581..15eff40f55f6 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -56,6 +56,7 @@ static inline enum nvdimm_security_state nvdimm_security_state(
 
 	return nvdimm->sec.ops->state(nvdimm);
 }
+int nvdimm_security_freeze(struct nvdimm *nvdimm);
 
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index f4d63f49f7dd..42c815f97c02 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -165,6 +165,7 @@ enum nvdimm_security_state {
 
 struct nvdimm_security_ops {
 	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
+	int (*freeze)(struct nvdimm *nvdimm);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 07/17] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (5 preceding siblings ...)
  2018-12-11 20:25 ` [PATCH v13 06/17] acpi/nfit, libnvdimm: Add freeze security support to Intel nvdimm Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 08/17] acpi/nfit, libnvdimm: Add disable passphrase support to Intel nvdimm Dave Jiang
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

From: Dan Williams <dan.j.williams@intel.com>

Add support to unlock the dimm via the kernel key management APIs. The
passphrase is expected to be pulled from userspace through keyutils.
The key management and sysfs attributes are libnvdimm generic.

Encrypted keys are used to protect the nvdimm passphrase at rest. The
master key can be a trusted-key sealed in a TPM, preferred, or an
encrypted-key, more flexible, but more exposure to a potential attacker.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/intel.c   |  108 +++++++++++++++++++++++++++++++
 drivers/nvdimm/Kconfig      |    4 +
 drivers/nvdimm/Makefile     |    1 
 drivers/nvdimm/dimm.c       |   16 ++++-
 drivers/nvdimm/nd.h         |    3 +
 drivers/nvdimm/security.c   |  148 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h   |   12 +++
 tools/testing/nvdimm/Kbuild |    1 
 8 files changed, 292 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvdimm/security.c

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index f98d680d1a39..5b627ca35d3b 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -75,8 +75,116 @@ static int intel_security_freeze(struct nvdimm *nvdimm)
 	return 0;
 }
 
+static int intel_security_change_key(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *old_data,
+		const struct nvdimm_key_data *new_data)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_set_passphrase cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_SET_PASSPHRASE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	if (old_data)
+		memcpy(nd_cmd.cmd.old_pass, old_data->data,
+				sizeof(nd_cmd.cmd.old_pass));
+	memcpy(nd_cmd.cmd.new_pass, new_data->data,
+			sizeof(nd_cmd.cmd.new_pass));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		return 0;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	case ND_INTEL_STATUS_NOT_SUPPORTED:
+		return -EOPNOTSUPP;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		return -EIO;
+	}
+}
+
+static void nvdimm_invalidate_cache(void);
+
+static int intel_security_unlock(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *key_data)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_unlock_unit cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_UNLOCK_UNIT,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	memcpy(nd_cmd.cmd.passphrase, key_data->data,
+			sizeof(nd_cmd.cmd.passphrase));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	default:
+		return -EIO;
+	}
+
+	/* DIMM unlocked, invalidate all CPU caches before we read it */
+	nvdimm_invalidate_cache();
+
+	return 0;
+}
+
+/*
+ * TODO: define a cross arch wbinvd equivalent when/if
+ * NVDIMM_FAMILY_INTEL command support arrives on another arch.
+ */
+#ifdef CONFIG_X86
+static void nvdimm_invalidate_cache(void)
+{
+	wbinvd_on_all_cpus();
+}
+#else
+static void nvdimm_invalidate_cache(void)
+{
+	WARN_ON_ONCE("cache invalidation required after unlock\n");
+}
+#endif
+
 static const struct nvdimm_security_ops __intel_security_ops = {
 	.state = intel_security_state,
 	.freeze = intel_security_freeze,
+	.change_key = intel_security_change_key,
+#ifdef CONFIG_X86
+	.unlock = intel_security_unlock,
+#endif
 };
+
 const struct nvdimm_security_ops *intel_security_ops = &__intel_security_ops;
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index 9d36473dc2a2..00f6325928f6 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -112,4 +112,8 @@ config OF_PMEM
 
 	  Select Y if unsure.
 
+config NVDIMM_KEYS
+        def_bool y
+        depends on KEYS
+
 endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index e8847045dac0..6f2a088afad6 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -27,3 +27,4 @@ libnvdimm-$(CONFIG_ND_CLAIM) += claim.o
 libnvdimm-$(CONFIG_BTT) += btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += dax_devs.o
+libnvdimm-$(CONFIG_NVDIMM_KEYS) += security.o
diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 9899c97138a3..1b3d9e7b2ffe 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -34,7 +34,11 @@ static int nvdimm_probe(struct device *dev)
 		return rc;
 	}
 
-	/* reset locked, to be validated below... */
+	/*
+	 * The locked status bit reflects explicit status codes from the
+	 * label reading commands, revalidate it each time the driver is
+	 * activated and re-reads the label area.
+	 */
 	nvdimm_clear_locked(dev);
 
 	ndd = kzalloc(sizeof(*ndd), GFP_KERNEL);
@@ -51,6 +55,16 @@ static int nvdimm_probe(struct device *dev)
 	get_device(dev);
 	kref_init(&ndd->kref);
 
+	/*
+	 * Attempt to unlock, if the DIMM supports security commands,
+	 * otherwise the locked indication is determined by explicit
+	 * status codes from the label reading commands.
+	 */
+	rc = nvdimm_security_unlock(dev);
+	if (rc < 0)
+		dev_err(dev, "failed to unlock dimm: %d\n", rc);
+
+
 	/*
 	 * EACCES failures reading the namespace label-area-properties
 	 * are interpreted as the DIMM capacity being locked but the
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index e79cc8e5c114..2bc1b011449a 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -250,6 +250,9 @@ 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);
+#if IS_ENABLED(CONFIG_NVDIMM_KEYS)
+int nvdimm_security_unlock(struct device *dev);
+#endif
 struct nd_btt *to_nd_btt(struct device *dev);
 
 struct nd_gen_sb {
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
new file mode 100644
index 000000000000..51d77a67a9fb
--- /dev/null
+++ b/drivers/nvdimm/security.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/ndctl.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/mm.h>
+#include <linux/cred.h>
+#include <linux/key.h>
+#include <linux/key-type.h>
+#include <keys/user-type.h>
+#include <keys/encrypted-type.h>
+#include "nd-core.h"
+#include "nd.h"
+
+static bool key_revalidate = true;
+module_param(key_revalidate, bool, 0444);
+MODULE_PARM_DESC(key_revalidate, "Require key validation at init.");
+
+static void *key_data(struct key *key)
+{
+	struct encrypted_key_payload *epayload = dereference_key_locked(key);
+
+	lockdep_assert_held_read(&key->sem);
+
+	return epayload->decrypted_data;
+}
+
+static void nvdimm_put_key(struct key *key)
+{
+	up_read(&key->sem);
+	key_put(key);
+}
+
+/*
+ * Retrieve kernel key for DIMM and request from user space if
+ * necessary. Returns a key held for read and must be put by
+ * nvdimm_put_key() before the usage goes out of scope.
+ */
+static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
+{
+	struct key *key = NULL;
+	static const char NVDIMM_PREFIX[] = "nvdimm:";
+	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
+	struct device *dev = &nvdimm->dev;
+
+	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
+	key = request_key(&key_type_encrypted, desc, "");
+	if (IS_ERR(key)) {
+		if (PTR_ERR(key) == -ENOKEY)
+			dev_warn(dev, "request_key() found no key\n");
+		else
+			dev_warn(dev, "request_key() upcall failed\n");
+		key = NULL;
+	} else {
+		struct encrypted_key_payload *epayload;
+
+		down_read(&key->sem);
+		epayload = dereference_key_locked(key);
+		if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) {
+			up_read(&key->sem);
+			key_put(key);
+			key = NULL;
+		}
+	}
+
+	return key;
+}
+
+static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
+{
+	struct key *key;
+	int rc;
+
+	if (!nvdimm->sec.ops->change_key)
+		return NULL;
+
+	key = nvdimm_request_key(nvdimm);
+	if (!key)
+		return NULL;
+
+	/*
+	 * Send the same key to the hardware as new and old key to
+	 * verify that the key is good.
+	 */
+	rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key), key_data(key));
+	if (rc < 0) {
+		nvdimm_put_key(key);
+		key = NULL;
+	}
+	return key;
+}
+
+static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key = NULL;
+	int rc;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	/*
+	 * If the pre-OS has unlocked the DIMM, attempt to send the key
+	 * from request_key() to the hardware for verification.  Failure
+	 * to revalidate the key against the hardware results in a
+	 * 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 (!key_revalidate)
+			return 0;
+
+		key = nvdimm_key_revalidate(nvdimm);
+		if (!key)
+			return nvdimm_security_freeze(nvdimm);
+	} else
+		key = nvdimm_request_key(nvdimm);
+
+	if (!key)
+		return -ENOKEY;
+
+	rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
+	dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	return rc;
+}
+
+int nvdimm_security_unlock(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	int rc;
+
+	nvdimm_bus_lock(dev);
+	rc = __nvdimm_security_unlock(nvdimm);
+	nvdimm_bus_unlock(dev);
+	return rc;
+}
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 42c815f97c02..0f0ab276134e 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -163,9 +163,21 @@ enum nvdimm_security_state {
 	NVDIMM_SECURITY_OVERWRITE,
 };
 
+#define NVDIMM_PASSPHRASE_LEN		32
+#define NVDIMM_KEY_DESC_LEN		22
+
+struct nvdimm_key_data {
+	u8 data[NVDIMM_PASSPHRASE_LEN];
+};
+
 struct nvdimm_security_ops {
 	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
 	int (*freeze)(struct nvdimm *nvdimm);
+	int (*change_key)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *old_data,
+			const struct nvdimm_key_data *new_data);
+	int (*unlock)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *key_data);
 };
 
 void badrange_init(struct badrange *badrange);
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 4a2f3cff2a75..33ea40777205 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -80,6 +80,7 @@ libnvdimm-$(CONFIG_ND_CLAIM) += $(NVDIMM_SRC)/claim.o
 libnvdimm-$(CONFIG_BTT) += $(NVDIMM_SRC)/btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += $(NVDIMM_SRC)/pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += $(NVDIMM_SRC)/dax_devs.o
+libnvdimm-$(CONFIG_NVDIMM_KEYS) += $(NVDIMM_SRC)/security.o
 libnvdimm-y += libnvdimm_test.o
 libnvdimm-y += config_check.o
 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 08/17] acpi/nfit, libnvdimm: Add disable passphrase support to Intel nvdimm.
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (6 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 07/17] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 09/17] acpi/nfit, libnvdimm: Add enable/update passphrase support for Intel nvdimms Dave Jiang
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add support to disable passphrase (security) for the Intel nvdimm. The
passphrase used for disabling is pulled from an encrypted-key in the kernel
user keyring. The action is triggered by writing "disable <keyid>" to the
sysfs attribute "security".

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/intel.c  |   44 +++++++++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   17 +++++++++++-
 drivers/nvdimm/nd-core.h   |    1 +
 drivers/nvdimm/security.c  |   63 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    2 +
 5 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 5b627ca35d3b..5d4557cfb0d7 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -162,6 +162,49 @@ static int intel_security_unlock(struct nvdimm *nvdimm,
 	return 0;
 }
 
+static int intel_security_disable(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *key_data)
+{
+	int rc;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_disable_passphrase cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_DISABLE_PASSPHRASE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+		.cmd = {
+			.status = 0,
+		},
+	};
+
+	if (!test_bit(NVDIMM_INTEL_DISABLE_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	memcpy(nd_cmd.cmd.passphrase, key_data->data,
+			sizeof(nd_cmd.cmd.passphrase));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
 /*
  * TODO: define a cross arch wbinvd equivalent when/if
  * NVDIMM_FAMILY_INTEL command support arrives on another arch.
@@ -182,6 +225,7 @@ static const struct nvdimm_security_ops __intel_security_ops = {
 	.state = intel_security_state,
 	.freeze = intel_security_freeze,
 	.change_key = intel_security_change_key,
+	.disable = intel_security_disable,
 #ifdef CONFIG_X86
 	.unlock = intel_security_unlock,
 #endif
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 8e0bd2ce4dd0..3a5727102a30 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -391,17 +391,32 @@ static ssize_t security_show(struct device *dev,
 	return -ENOTTY;
 }
 
+#define SEC_CMD_SIZE 32
+#define KEY_ID_SIZE 10 /* u32 size in string */
 static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
 	ssize_t rc;
+	char cmd[SEC_CMD_SIZE+1], keystr[KEY_ID_SIZE+1];
+	unsigned int key;
 
 	if (atomic_read(&nvdimm->busy))
 		return -EBUSY;
 
+	rc = sscanf(buf, "%"__stringify(SEC_CMD_SIZE)"s %"__stringify(KEY_ID_SIZE)"s",
+			cmd, keystr);
 	if (sysfs_streq(buf, "freeze")) {
 		dev_dbg(dev, "freeze\n");
 		rc = nvdimm_security_freeze(nvdimm);
+	} else if (sysfs_streq(cmd, "disable")) {
+		if (rc != 2)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_disable(nvdimm, key);
+		if (rc < 0)
+			return rc;
 	} else
 		return -EINVAL;
 
@@ -452,7 +467,7 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
 	if (nvdimm->sec.state < 0)
 		return 0;
 	/* Are there any state mutation ops? */
-	if (nvdimm->sec.ops->freeze)
+	if (nvdimm->sec.ops->freeze || nvdimm->sec.ops->disable)
 		return a->mode;
 	return 0444;
 }
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 15eff40f55f6..93e63c12ea45 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -57,6 +57,7 @@ static inline enum nvdimm_security_state nvdimm_security_state(
 	return nvdimm->sec.ops->state(nvdimm);
 }
 int nvdimm_security_freeze(struct nvdimm *nvdimm);
+int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
 
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 51d77a67a9fb..55d8806a5040 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -69,6 +69,36 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 	return key;
 }
 
+static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
+		key_serial_t id)
+{
+	key_ref_t keyref;
+	struct key *key;
+	struct encrypted_key_payload *epayload;
+	struct device *dev = &nvdimm->dev;
+
+	keyref = lookup_user_key(id, 0, 0);
+	if (IS_ERR(keyref))
+		return NULL;
+
+	key = key_ref_to_ptr(keyref);
+	if (key->type != &key_type_encrypted) {
+		key_put(key);
+		return NULL;
+	}
+	dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key));
+
+
+	down_read(&key->sem);
+	epayload = dereference_key_locked(key);
+	if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) {
+		up_read(&key->sem);
+		key_put(key);
+		key = NULL;
+	}
+	return key;
+}
+
 static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
 {
 	struct key *key;
@@ -146,3 +176,36 @@ int nvdimm_security_unlock(struct device *dev)
 	nvdimm_bus_unlock(dev);
 	return rc;
 }
+
+int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	int rc;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->disable
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
+		dev_warn(dev, "Incorrect security state: %d\n",
+				nvdimm->sec.state);
+		return -EIO;
+	}
+
+	key = nvdimm_lookup_user_key(nvdimm, keyid);
+	if (!key)
+		return -ENOKEY;
+
+	rc = nvdimm->sec.ops->disable(nvdimm, key_data(key));
+	dev_dbg(dev, "key: %d disable: %s\n", key_serial(key),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	return rc;
+}
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 0f0ab276134e..d0afa115356e 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -178,6 +178,8 @@ struct nvdimm_security_ops {
 			const struct nvdimm_key_data *new_data);
 	int (*unlock)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
+	int (*disable)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *key_data);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 09/17] acpi/nfit, libnvdimm: Add enable/update passphrase support for Intel nvdimms
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (7 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 08/17] acpi/nfit, libnvdimm: Add disable passphrase support to Intel nvdimm Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 10/17] acpi/nfit, libnvdimm: Add support for issue secure erase DSM to Intel nvdimm Dave Jiang
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add support for enabling and updating passphrase on the Intel nvdimms.
The passphrase is the an encrypted key in the kernel user keyring.
We trigger the update via writing "update <old_keyid> <new_keyid>" to the
sysfs attribute "security". If no <old_keyid> exists (for enabling
security) then a 0 should be used.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/dimm_devs.c |   24 ++++++++++++++----
 drivers/nvdimm/nd-core.h   |    2 ++
 drivers/nvdimm/security.c  |   58 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 3a5727102a30..8669000ea6a6 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -397,14 +397,15 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
 	ssize_t rc;
-	char cmd[SEC_CMD_SIZE+1], keystr[KEY_ID_SIZE+1];
-	unsigned int key;
+	char cmd[SEC_CMD_SIZE+1], keystr[KEY_ID_SIZE+1],
+		nkeystr[KEY_ID_SIZE+1];
+	unsigned int key, newkey;
 
 	if (atomic_read(&nvdimm->busy))
 		return -EBUSY;
 
-	rc = sscanf(buf, "%"__stringify(SEC_CMD_SIZE)"s %"__stringify(KEY_ID_SIZE)"s",
-			cmd, keystr);
+	rc = sscanf(buf, "%"__stringify(SEC_CMD_SIZE)"s %"__stringify(KEY_ID_SIZE)"s %"__stringify(KEY_ID_SIZE)"s",
+			cmd, keystr, nkeystr);
 	if (sysfs_streq(buf, "freeze")) {
 		dev_dbg(dev, "freeze\n");
 		rc = nvdimm_security_freeze(nvdimm);
@@ -417,6 +418,18 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = nvdimm_security_disable(nvdimm, key);
 		if (rc < 0)
 			return rc;
+	} else if (sysfs_streq(cmd, "update")) {
+		if (rc != 3)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = kstrtouint(nkeystr, 0, &newkey);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_update(nvdimm, key, newkey);
+		if (rc < 0)
+			return rc;
 	} else
 		return -EINVAL;
 
@@ -467,7 +480,8 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
 	if (nvdimm->sec.state < 0)
 		return 0;
 	/* Are there any state mutation ops? */
-	if (nvdimm->sec.ops->freeze || nvdimm->sec.ops->disable)
+	if (nvdimm->sec.ops->freeze || nvdimm->sec.ops->disable
+			|| nvdimm->sec.ops->change_key)
 		return a->mode;
 	return 0444;
 }
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 93e63c12ea45..ca020793a419 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -58,6 +58,8 @@ static inline enum nvdimm_security_state nvdimm_security_state(
 }
 int nvdimm_security_freeze(struct nvdimm *nvdimm);
 int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
+int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
+		unsigned int new_keyid);
 
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 55d8806a5040..654b64fe7e9d 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -15,6 +15,9 @@
 #include "nd-core.h"
 #include "nd.h"
 
+#define NVDIMM_BASE_KEY		0
+#define NVDIMM_NEW_KEY		1
+
 static bool key_revalidate = true;
 module_param(key_revalidate, bool, 0444);
 MODULE_PARM_DESC(key_revalidate, "Require key validation at init.");
@@ -70,7 +73,7 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 }
 
 static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
-		key_serial_t id)
+		key_serial_t id, int subclass)
 {
 	key_ref_t keyref;
 	struct key *key;
@@ -86,10 +89,10 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
 		key_put(key);
 		return NULL;
 	}
-	dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key));
 
+	dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key));
 
-	down_read(&key->sem);
+	down_read_nested(&key->sem, subclass);
 	epayload = dereference_key_locked(key);
 	if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) {
 		up_read(&key->sem);
@@ -197,7 +200,7 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
 		return -EIO;
 	}
 
-	key = nvdimm_lookup_user_key(nvdimm, keyid);
+	key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
 	if (!key)
 		return -ENOKEY;
 
@@ -209,3 +212,50 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
 	nvdimm->sec.state = nvdimm_security_state(nvdimm);
 	return rc;
 }
+
+int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
+		unsigned int new_keyid)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key, *newkey;
+	int rc;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->change_key
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
+		dev_warn(dev, "Incorrect security state: %d\n",
+				nvdimm->sec.state);
+		return -EIO;
+	}
+
+	if (keyid == 0)
+		key = NULL;
+	else {
+		key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
+		if (!key)
+			return -ENOKEY;
+	}
+
+	newkey = nvdimm_lookup_user_key(nvdimm, new_keyid, NVDIMM_NEW_KEY);
+	if (!newkey) {
+		nvdimm_put_key(key);
+		return -ENOKEY;
+	}
+
+	rc = nvdimm->sec.ops->change_key(nvdimm, key ? key_data(key) : NULL,
+			key_data(newkey));
+	dev_dbg(dev, "key: %d %d update: %s\n",
+			key_serial(key), key_serial(newkey),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(newkey);
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	return rc;
+}

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 10/17] acpi/nfit, libnvdimm: Add support for issue secure erase DSM to Intel nvdimm
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (8 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 09/17] acpi/nfit, libnvdimm: Add enable/update passphrase support for Intel nvdimms Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 11/17] libnvdimm/security: introduce NDD_SECURITY_BUSY flag Dave Jiang
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add support to issue a secure erase DSM to the Intel nvdimm. The
required passphrase is acquired from an encrypted key in the kernel user
keyring. To trigger the action, "erase <keyid>" is written to the
"security" sysfs attribute.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/intel.c  |   50 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   12 ++++++++++-
 drivers/nvdimm/nd-core.h   |    1 +
 drivers/nvdimm/security.c  |   41 ++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    2 ++
 5 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 5d4557cfb0d7..ce65111ea4e1 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -205,6 +205,55 @@ static int intel_security_disable(struct nvdimm *nvdimm,
 	return 0;
 }
 
+static int intel_security_erase(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *key)
+{
+	int rc;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_secure_erase cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+			.nd_command = NVDIMM_INTEL_SECURE_ERASE,
+		},
+		.cmd = {
+			.status = 0,
+		},
+	};
+
+	if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	/* flush all cache before we erase DIMM */
+	nvdimm_invalidate_cache();
+	memcpy(nd_cmd.cmd.passphrase, key->data,
+			sizeof(nd_cmd.cmd.passphrase));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_NOT_SUPPORTED:
+		return -EOPNOTSUPP;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		return -ENXIO;
+	}
+
+	/* DIMM erased, invalidate all CPU caches before we read it */
+	nvdimm_invalidate_cache();
+	return 0;
+}
+
 /*
  * TODO: define a cross arch wbinvd equivalent when/if
  * NVDIMM_FAMILY_INTEL command support arrives on another arch.
@@ -228,6 +277,7 @@ static const struct nvdimm_security_ops __intel_security_ops = {
 	.disable = intel_security_disable,
 #ifdef CONFIG_X86
 	.unlock = intel_security_unlock,
+	.erase = intel_security_erase,
 #endif
 };
 
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 8669000ea6a6..e3b46849ee0a 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -430,6 +430,15 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = nvdimm_security_update(nvdimm, key, newkey);
 		if (rc < 0)
 			return rc;
+	} else if (sysfs_streq(cmd, "erase")) {
+		if (rc != 2)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_erase(nvdimm, key);
+		if (rc < 0)
+			return rc;
 	} else
 		return -EINVAL;
 
@@ -481,7 +490,8 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
 		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->change_key
+			|| nvdimm->sec.ops->erase)
 		return a->mode;
 	return 0444;
 }
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index ca020793a419..3c8cdd40c456 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -60,6 +60,7 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm);
 int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
 int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
 		unsigned int new_keyid);
+int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid);
 
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 654b64fe7e9d..4836f2fda271 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -33,6 +33,9 @@ static void *key_data(struct key *key)
 
 static void nvdimm_put_key(struct key *key)
 {
+	if (!key)
+		return;
+
 	up_read(&key->sem);
 	key_put(key);
 }
@@ -259,3 +262,41 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
 	nvdimm->sec.state = nvdimm_security_state(nvdimm);
 	return rc;
 }
+
+int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	int rc;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->erase
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	if (atomic_read(&nvdimm->busy)) {
+		dev_warn(dev, "Unable to secure erase while DIMM active.\n");
+		return -EBUSY;
+	}
+
+	if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
+		dev_warn(dev, "Incorrect security state: %d\n",
+				nvdimm->sec.state);
+		return -EIO;
+	}
+
+	key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
+	if (!key)
+		return -ENOKEY;
+
+	rc = nvdimm->sec.ops->erase(nvdimm, key_data(key));
+	dev_dbg(dev, "key: %d erase: %s\n", key_serial(key),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	return rc;
+}
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index d0afa115356e..9a6cb7067dc7 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -180,6 +180,8 @@ struct nvdimm_security_ops {
 			const struct nvdimm_key_data *key_data);
 	int (*disable)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
+	int (*erase)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *key_data);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 11/17] libnvdimm/security: introduce NDD_SECURITY_BUSY flag
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (9 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 10/17] acpi/nfit, libnvdimm: Add support for issue secure erase DSM to Intel nvdimm Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support Dave Jiang
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Adding a flag for nvdimm->flags to support erase functions. While it's ok
to hold the nvdimm_bus lock for secure erase due to minimal time to execute
the command, overwrite requires a significantly longer time and makes this
impossible. The flag will block any drivers from being loaded and DIMMs
being probed.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/nvdimm/dimm_devs.c   |    6 ++++++
 drivers/nvdimm/nd-core.h     |   16 ++++++++++++++++
 drivers/nvdimm/region_devs.c |    7 +++++++
 drivers/nvdimm/security.c    |   18 ++++++++++++++++++
 include/linux/libnvdimm.h    |    2 ++
 5 files changed, 49 insertions(+)

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index e3b46849ee0a..bfdc4824ba11 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -557,6 +557,12 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
 	if (nvdimm->sec.state < 0)
 		return -EIO;
 
+	rc = nvdimm_security_check_busy(nvdimm);
+	if (rc < 0) {
+		dev_warn(&nvdimm->dev, "Security operation in progress.\n");
+		return rc;
+	}
+
 	rc = nvdimm->sec.ops->freeze(nvdimm);
 	nvdimm->sec.state = nvdimm_security_state(nvdimm);
 
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 3c8cdd40c456..353f945cda5b 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -61,6 +61,22 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
 int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
 		unsigned int new_keyid);
 int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid);
+static inline int nvdimm_security_check_busy(struct nvdimm *nvdimm)
+{
+	if (test_bit(NDD_SECURITY_BUSY, &nvdimm->flags))
+		return -EBUSY;
+	return 0;
+}
+
+static inline void nvdimm_set_security_busy(struct nvdimm *nvdimm)
+{
+	set_bit(NDD_SECURITY_BUSY, &nvdimm->flags);
+}
+
+static inline void nvdimm_clear_security_busy(struct nvdimm *nvdimm)
+{
+	clear_bit(NDD_SECURITY_BUSY, &nvdimm->flags);
+}
 
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 174a418cb171..7ccca49c7a52 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -78,6 +78,13 @@ int nd_region_activate(struct nd_region *nd_region)
 	for (i = 0; i < nd_region->ndr_mappings; i++) {
 		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
 		struct nvdimm *nvdimm = nd_mapping->nvdimm;
+		int rc;
+
+		rc = nvdimm_security_check_busy(nvdimm);
+		if (rc) {
+			nvdimm_bus_unlock(&nd_region->dev);
+			return rc;
+		}
 
 		/* at least one null hint slot per-dimm for the "no-hint" case */
 		flush_data_size += sizeof(void *);
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 4836f2fda271..5a03dffd0056 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -143,6 +143,12 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
 			|| nvdimm->sec.state < 0)
 		return -EIO;
 
+	rc = nvdimm_security_check_busy(nvdimm);
+	if (rc < 0) {
+		dev_warn(dev, "Security operation in progress.\n");
+		return rc;
+	}
+
 	/*
 	 * If the pre-OS has unlocked the DIMM, attempt to send the key
 	 * from request_key() to the hardware for verification.  Failure
@@ -203,6 +209,12 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
 		return -EIO;
 	}
 
+	rc = nvdimm_security_check_busy(nvdimm);
+	if (rc < 0) {
+		dev_warn(dev, "Security operation in progress.\n");
+		return rc;
+	}
+
 	key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
 	if (!key)
 		return -ENOKEY;
@@ -288,6 +300,12 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
 		return -EIO;
 	}
 
+	rc = nvdimm_security_check_busy(nvdimm);
+	if (rc < 0) {
+		dev_warn(dev, "Security operation in progress.\n");
+		return rc;
+	}
+
 	key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
 	if (!key)
 		return -ENOKEY;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 9a6cb7067dc7..8507d2896ae0 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -38,6 +38,8 @@ enum {
 	NDD_UNARMED = 1,
 	/* locked memory devices should not be accessed */
 	NDD_LOCKED = 2,
+	/* memory under security wipes should not be accessed */
+	NDD_SECURITY_BUSY = 3,
 
 	/* need to set a limit somewhere, but yes, this is likely overkill */
 	ND_IOCTL_MAX_BUFLEN = SZ_4M,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (10 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 11/17] libnvdimm/security: introduce NDD_SECURITY_BUSY flag Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 23:44   ` Dan Williams
  2018-12-11 20:26 ` [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support Dave Jiang
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

We are adding support for the security calls of ovewrite and query
overwrite introduced from Intel DSM spec v1.7. This will allow triggering
of overwrite on Intel NVDIMMs. The overwrite operation can take tens
of minutes. When the overwrite DSM is issued successfully, the NVDIMMs
will be unaccessible. The kernel will do backoff polling to detect when
the overwrite process is completed. According to the DSM spec v1.7,
the 128G NVDIMMs can take up to 15mins to perform overwrite and larger
DIMMs will take longer.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/core.c   |    5 ++
 drivers/acpi/nfit/intel.c  |   93 ++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/nfit.h   |    1 
 drivers/nvdimm/core.c      |    3 +
 drivers/nvdimm/dimm_devs.c |   35 +++++++++++++++-
 drivers/nvdimm/nd-core.h   |    8 ++++
 drivers/nvdimm/security.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    4 ++
 8 files changed, 245 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 77f188cd8023..173517eb35b1 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2043,6 +2043,11 @@ 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/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index ce65111ea4e1..3c3f99dfc32c 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -254,6 +254,97 @@ static int intel_security_erase(struct nvdimm *nvdimm,
 	return 0;
 }
 
+static int intel_security_query_overwrite(struct nvdimm *nvdimm)
+{
+	int rc;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_query_overwrite cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_QUERY_OVERWRITE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = 0,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+		.cmd = {
+			.status = 0,
+		},
+	};
+
+	if (!test_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_OQUERY_INPROGRESS:
+		return -EBUSY;
+	default:
+		return -ENXIO;
+	}
+
+	/* flush all cache before we make the nvdimms available */
+	nvdimm_invalidate_cache();
+	nfit_mem->overwrite = false;
+	return 0;
+}
+
+static int intel_security_overwrite(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *nkey)
+{
+	int rc;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_overwrite cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_OVERWRITE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+		.cmd = {
+			.status = 0,
+		},
+	};
+
+	if (!test_bit(NVDIMM_INTEL_OVERWRITE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	/* flush all cache before we erase DIMM */
+	nvdimm_invalidate_cache();
+	if (nkey)
+		memcpy(nd_cmd.cmd.passphrase, nkey->data,
+				sizeof(nd_cmd.cmd.passphrase));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		nfit_mem->overwrite = true;
+		break;
+	case ND_INTEL_STATUS_OVERWRITE_UNSUPPORTED:
+		return -ENOTSUPP;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
 /*
  * TODO: define a cross arch wbinvd equivalent when/if
  * NVDIMM_FAMILY_INTEL command support arrives on another arch.
@@ -278,6 +369,8 @@ static const struct nvdimm_security_ops __intel_security_ops = {
 #ifdef CONFIG_X86
 	.unlock = intel_security_unlock,
 	.erase = intel_security_erase,
+	.overwrite = intel_security_overwrite,
+	.query_overwrite = intel_security_query_overwrite,
 #endif
 };
 
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index 33691aecfcee..e0ee54049c89 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -208,6 +208,7 @@ struct nfit_mem {
 	unsigned long flags;
 	u32 dirty_shutdown;
 	int family;
+	bool overwrite;
 };
 
 struct acpi_nfit_desc {
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index acce050856a8..b2496c06178b 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -437,6 +437,9 @@ static __init int libnvdimm_init(void)
 {
 	int rc;
 
+	rc = nvdimm_devs_init();
+	if (rc)
+		return rc;
 	rc = nvdimm_bus_init();
 	if (rc)
 		return rc;
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index bfdc4824ba11..fa42774efb15 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -24,6 +24,7 @@
 #include "nd.h"
 
 static DEFINE_IDA(dimm_ida);
+struct workqueue_struct *nvdimm_wq;
 
 /*
  * Retrieve bus and dimm handle and return if this bus supports
@@ -439,6 +440,15 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = nvdimm_security_erase(nvdimm, key);
 		if (rc < 0)
 			return rc;
+	} else if (sysfs_streq(cmd, "overwite")) {
+		if (rc != 2)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_overwrite(nvdimm, key);
+		if (rc < 0)
+			return rc;
 	} else
 		return -EINVAL;
 
@@ -491,7 +501,8 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
 	/* 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->erase
+			|| nvdimm->sec.ops->overwrite)
 		return a->mode;
 	return 0444;
 }
@@ -534,6 +545,8 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 	dev->devt = MKDEV(nvdimm_major, nvdimm->id);
 	dev->groups = groups;
 	nvdimm->sec.ops = sec_ops;
+	nvdimm->sec.overwrite_tmo = 0;
+	INIT_DELAYED_WORK(&nvdimm->dwork, nvdimm_security_overwrite_query);
 	/*
 	 * Security state must be initialized before device_add() for
 	 * attribute visibility.
@@ -545,6 +558,16 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 }
 EXPORT_SYMBOL_GPL(__nvdimm_create);
 
+int nvdimm_security_setup_events(struct nvdimm *nvdimm)
+{
+	nvdimm->sec.overwrite_state = sysfs_get_dirent(nvdimm->dev.kobj.sd,
+			"security");
+	if (!nvdimm->sec.overwrite_state)
+		return -ENODEV;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(nvdimm_security_setup_events);
+
 int nvdimm_security_freeze(struct nvdimm *nvdimm)
 {
 	int rc;
@@ -839,7 +862,17 @@ int nvdimm_bus_check_dimm_count(struct nvdimm_bus *nvdimm_bus, int dimm_count)
 }
 EXPORT_SYMBOL_GPL(nvdimm_bus_check_dimm_count);
 
+int __init nvdimm_devs_init(void)
+{
+	nvdimm_wq = create_singlethread_workqueue("nvdimm");
+	if (!nvdimm_wq)
+		return -ENOMEM;
+
+	return 0;
+}
+
 void __exit nvdimm_devs_exit(void)
 {
+	destroy_workqueue(nvdimm_wq);
 	ida_destroy(&dimm_ida);
 }
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 353f945cda5b..2c93e2139346 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -21,6 +21,7 @@
 extern struct list_head nvdimm_bus_list;
 extern struct mutex nvdimm_bus_list_mutex;
 extern int nvdimm_major;
+extern struct workqueue_struct *nvdimm_wq;
 
 struct nvdimm_bus {
 	struct nvdimm_bus_descriptor *nd_desc;
@@ -45,7 +46,10 @@ struct nvdimm {
 	struct {
 		const struct nvdimm_security_ops *ops;
 		enum nvdimm_security_state state;
+		unsigned int overwrite_tmo;
+		struct kernfs_node *overwrite_state;
 	} sec;
+	struct delayed_work dwork;
 };
 
 static inline enum nvdimm_security_state nvdimm_security_state(
@@ -78,6 +82,9 @@ static inline void nvdimm_clear_security_busy(struct nvdimm *nvdimm)
 	clear_bit(NDD_SECURITY_BUSY, &nvdimm->flags);
 }
 
+int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid);
+void nvdimm_security_overwrite_query(struct work_struct *work);
+
 /**
  * struct blk_alloc_info - tracking info for BLK dpa scanning
  * @nd_mapping: blk region mapping boundaries
@@ -110,6 +117,7 @@ static inline bool is_memory(struct device *dev)
 struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev);
 int __init nvdimm_bus_init(void);
 void nvdimm_bus_exit(void);
+int nvdimm_devs_init(void);
 void nvdimm_devs_exit(void);
 void nd_region_devs_exit(void);
 void nd_region_probe_success(struct nvdimm_bus *nvdimm_bus, struct device *dev);
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 5a03dffd0056..3603c5fda1cf 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -282,7 +282,7 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
 	struct key *key;
 	int rc;
 
-	/* The bus lock should be held at the top level of the call stack */
+	/* the bus lock should be held at the top level of the call stack */
 	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
 
 	if (!nvdimm->sec.ops || !nvdimm->sec.ops->erase
@@ -318,3 +318,99 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
 	nvdimm->sec.state = nvdimm_security_state(nvdimm);
 	return rc;
 }
+
+int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	int rc;
+
+	/* the bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->overwrite
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	if (atomic_read(&nvdimm->busy)) {
+		dev_warn(dev, "Unable to overwrite while DIMM active.\n");
+		nvdimm_clear_security_busy(nvdimm);
+		return -EBUSY;
+	}
+
+	if (dev_get_drvdata(dev)) {
+		dev_warn(dev, "Unable to overwrite while DIMM active.\n");
+		nvdimm_clear_security_busy(nvdimm);
+		return -EINVAL;
+	}
+
+	if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
+		dev_warn(dev, "Incorrect security state: %d\n",
+				nvdimm->sec.state);
+		return -EIO;
+	}
+
+	rc = nvdimm_security_check_busy(nvdimm);
+	if (rc < 0) {
+		dev_warn(dev, "Security operation in progress.\n");
+		return rc;
+	}
+
+	if (keyid == 0)
+		key = NULL;
+	else {
+		key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
+		if (!key)
+			return -ENOKEY;
+	}
+
+	rc = nvdimm->sec.ops->overwrite(nvdimm, key ? key_data(key) : NULL);
+	dev_dbg(dev, "key: %d overwrite submission: %s\n", key_serial(key),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	nvdimm_set_security_busy(nvdimm);
+	return rc;
+}
+
+void nvdimm_security_overwrite_query(struct work_struct *work)
+{
+	struct nvdimm_bus *nvdimm_bus;
+	struct nvdimm *nvdimm;
+	int rc;
+	unsigned int tmo;
+
+	nvdimm = container_of(work, typeof(*nvdimm), dwork.work);
+	nvdimm_bus = walk_to_nvdimm_bus(&nvdimm->dev);
+	tmo = nvdimm->sec.overwrite_tmo;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->query_overwrite
+			|| nvdimm->sec.state < 0)
+		return;
+
+	rc = nvdimm->sec.ops->query_overwrite(nvdimm);
+	if (rc == -EBUSY) {
+
+		/* setup delayed work again */
+		tmo += 10;
+		queue_delayed_work(nvdimm_wq, &nvdimm->dwork, tmo * HZ);
+		nvdimm->sec.overwrite_tmo = min(15U * 60U, tmo);
+		return;
+	}
+
+	if (rc < 0)
+		dev_warn(&nvdimm->dev, "overwrite failed\n");
+	else
+		dev_info(&nvdimm->dev, "overwrite completed\n");
+
+	if (nvdimm->sec.overwrite_state)
+		sysfs_notify_dirent(nvdimm->sec.overwrite_state);
+	nvdimm->sec.overwrite_tmo = 0;
+	nvdimm_clear_security_busy(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+}
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 8507d2896ae0..b728952ccb28 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -184,6 +184,9 @@ struct nvdimm_security_ops {
 			const struct nvdimm_key_data *key_data);
 	int (*erase)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
+	int (*overwrite)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *key_data);
+	int (*query_overwrite)(struct nvdimm *nvdimm);
 };
 
 void badrange_init(struct badrange *badrange);
@@ -221,6 +224,7 @@ 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,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (11 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 23:30   ` Dan Williams
  2018-12-11 20:26 ` [PATCH v13 14/17] tools/testing/nvdimm: Add test support for Intel nvdimm security DSMs Dave Jiang
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

With Intel DSM 1.8 [1] two new security DSMs are introduced. Enable/update
master passphrase and master secure erase. The master passphrase allows
a secure erase to be performed without the user passphrase that is set on
the NVDIMM. The commands of master_update and master_erase are added to
the sysfs knob in order to initiate the DSMs. They are similar in opeartion
mechanism compare to update and erase.

[1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/core.c   |    2 +
 drivers/acpi/nfit/intel.c  |   61 ++++++++++++++++++++++++++++++++------------
 drivers/nvdimm/dimm_devs.c |   33 +++++++++++++++++++++---
 drivers/nvdimm/nd-core.h   |   10 ++++---
 drivers/nvdimm/security.c  |   41 ++++++++++++++++++++----------
 include/linux/libnvdimm.h  |    9 ++++--
 6 files changed, 115 insertions(+), 41 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 173517eb35b1..2e92b9d51c38 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -389,6 +389,8 @@ static u8 nfit_dsm_revid(unsigned family, unsigned func)
 			[NVDIMM_INTEL_SECURE_ERASE] = 2,
 			[NVDIMM_INTEL_OVERWRITE] = 2,
 			[NVDIMM_INTEL_QUERY_OVERWRITE] = 2,
+			[NVDIMM_INTEL_SET_MASTER_PASSPHRASE] = 2,
+			[NVDIMM_INTEL_MASTER_SECURE_ERASE] = 2,
 		},
 	};
 	u8 id;
diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 3c3f99dfc32c..09317ea23e72 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -6,7 +6,8 @@
 #include "intel.h"
 #include "nfit.h"
 
-static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
+static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm,
+		bool master)
 {
 	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
 	struct {
@@ -34,17 +35,28 @@ static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
 		return -EIO;
 
 	/* check and see if security is enabled and locked */
-	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;
+	if (master) {
+		if (nd_cmd.cmd.extended_state & ND_INTEL_SEC_ESTATE_ENABLED)
+			return NVDIMM_SECURITY_MASTER_ENABLED;
+		else if (nd_cmd.cmd.extended_state &
+				ND_INTEL_SEC_ESTATE_PLIMIT)
+			return NVDIMM_SECURITY_MASTER_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;
+		}
 	}
+
+	/* this should cover master security disabled as well */
 	return NVDIMM_SECURITY_DISABLED;
 }
 
@@ -77,7 +89,8 @@ static int intel_security_freeze(struct nvdimm *nvdimm)
 
 static int intel_security_change_key(struct nvdimm *nvdimm,
 		const struct nvdimm_key_data *old_data,
-		const struct nvdimm_key_data *new_data)
+		const struct nvdimm_key_data *new_data,
+		bool master)
 {
 	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
 	struct {
@@ -85,7 +98,6 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
 		struct nd_intel_set_passphrase cmd;
 	} nd_cmd = {
 		.pkg = {
-			.nd_command = NVDIMM_INTEL_SET_PASSPHRASE,
 			.nd_family = NVDIMM_FAMILY_INTEL,
 			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2,
 			.nd_size_out = ND_INTEL_STATUS_SIZE,
@@ -94,7 +106,15 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
 	};
 	int rc;
 
-	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
+	if (master)
+		nd_cmd.pkg.nd_command = NVDIMM_INTEL_SET_MASTER_PASSPHRASE;
+	else
+		nd_cmd.pkg.nd_command = NVDIMM_INTEL_SET_PASSPHRASE;
+
+	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask) ||
+			(master &&
+			 !test_bit(NVDIMM_INTEL_SET_MASTER_PASSPHRASE,
+				 &nfit_mem->dsm_mask)))
 		return -ENOTTY;
 
 	if (old_data)
@@ -206,7 +226,7 @@ static int intel_security_disable(struct nvdimm *nvdimm,
 }
 
 static int intel_security_erase(struct nvdimm *nvdimm,
-		const struct nvdimm_key_data *key)
+		const struct nvdimm_key_data *key, bool master)
 {
 	int rc;
 	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
@@ -219,14 +239,21 @@ static int intel_security_erase(struct nvdimm *nvdimm,
 			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
 			.nd_size_out = ND_INTEL_STATUS_SIZE,
 			.nd_fw_size = ND_INTEL_STATUS_SIZE,
-			.nd_command = NVDIMM_INTEL_SECURE_ERASE,
 		},
 		.cmd = {
 			.status = 0,
 		},
 	};
 
-	if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask))
+	if (master)
+		nd_cmd.pkg.nd_command = NVDIMM_INTEL_MASTER_SECURE_ERASE;
+	else
+		nd_cmd.pkg.nd_command = NVDIMM_INTEL_SECURE_ERASE;
+
+	if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask) ||
+			(master &&
+			 !test_bit(NVDIMM_INTEL_MASTER_SECURE_ERASE,
+				 &nfit_mem->dsm_mask)))
 		return -ENOTTY;
 
 	/* flush all cache before we erase DIMM */
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index fa42774efb15..8825551aad84 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -387,6 +387,8 @@ static ssize_t security_show(struct device *dev,
 		return sprintf(buf, "frozen\n");
 	case NVDIMM_SECURITY_OVERWRITE:
 		return sprintf(buf, "overwrite\n");
+	default:
+		return -ENOTTY;
 	}
 
 	return -ENOTTY;
@@ -428,7 +430,7 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = kstrtouint(nkeystr, 0, &newkey);
 		if (rc < 0)
 			return rc;
-		rc = nvdimm_security_update(nvdimm, key, newkey);
+		rc = nvdimm_security_update(nvdimm, key, newkey, false);
 		if (rc < 0)
 			return rc;
 	} else if (sysfs_streq(cmd, "erase")) {
@@ -437,7 +439,7 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = kstrtouint(keystr, 0, &key);
 		if (rc < 0)
 			return rc;
-		rc = nvdimm_security_erase(nvdimm, key);
+		rc = nvdimm_security_erase(nvdimm, key, false);
 		if (rc < 0)
 			return rc;
 	} else if (sysfs_streq(cmd, "overwite")) {
@@ -449,6 +451,27 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
 		rc = nvdimm_security_overwrite(nvdimm, key);
 		if (rc < 0)
 			return rc;
+	} else if (sysfs_streq(cmd, "master_update")) {
+		if (rc != 3)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = kstrtouint(nkeystr, 0, &newkey);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_update(nvdimm, key, newkey, true);
+		if (rc < 0)
+			return rc;
+	} else if (sysfs_streq(cmd, "master_erase")) {
+		if (rc != 2)
+			return -EINVAL;
+		rc = kstrtouint(keystr, 0, &key);
+		if (rc < 0)
+			return rc;
+		rc = nvdimm_security_erase(nvdimm, key, true);
+		if (rc < 0)
+			return rc;
 	} else
 		return -EINVAL;
 
@@ -551,7 +574,9 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
 	 * Security state must be initialized before device_add() for
 	 * attribute visibility.
 	 */
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	/* get security state and extended (master) state */
+	nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
+	nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
 	nd_device_register(dev);
 
 	return nvdimm;
@@ -587,7 +612,7 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
 	}
 
 	rc = nvdimm->sec.ops->freeze(nvdimm);
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
 
 	return rc;
 }
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 2c93e2139346..f3e05e32c530 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -46,6 +46,7 @@ struct nvdimm {
 	struct {
 		const struct nvdimm_security_ops *ops;
 		enum nvdimm_security_state state;
+		enum nvdimm_security_state ext_state;
 		unsigned int overwrite_tmo;
 		struct kernfs_node *overwrite_state;
 	} sec;
@@ -53,18 +54,19 @@ struct nvdimm {
 };
 
 static inline enum nvdimm_security_state nvdimm_security_state(
-		struct nvdimm *nvdimm)
+		struct nvdimm *nvdimm, bool master)
 {
 	if (!nvdimm->sec.ops)
 		return -ENXIO;
 
-	return nvdimm->sec.ops->state(nvdimm);
+	return nvdimm->sec.ops->state(nvdimm, master);
 }
 int nvdimm_security_freeze(struct nvdimm *nvdimm);
 int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
 int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
-		unsigned int new_keyid);
-int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid);
+		unsigned int new_keyid, bool master);
+int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
+		bool master);
 static inline int nvdimm_security_check_busy(struct nvdimm *nvdimm)
 {
 	if (test_bit(NDD_SECURITY_BUSY, &nvdimm->flags))
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 3603c5fda1cf..a10eaf4e23c3 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -121,7 +121,8 @@ static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
 	 * Send the same key to the hardware as new and old key to
 	 * verify that the key is good.
 	 */
-	rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key), key_data(key));
+	rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key),
+			key_data(key), false);
 	if (rc < 0) {
 		nvdimm_put_key(key);
 		key = NULL;
@@ -174,7 +175,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->sec.state = nvdimm_security_state(nvdimm, false);
 	return rc;
 }
 
@@ -224,12 +225,12 @@ 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->sec.state = nvdimm_security_state(nvdimm, false);
 	return rc;
 }
 
 int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
-		unsigned int new_keyid)
+		unsigned int new_keyid, bool master)
 {
 	struct device *dev = &nvdimm->dev;
 	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
@@ -264,18 +265,23 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
 	}
 
 	rc = nvdimm->sec.ops->change_key(nvdimm, key ? key_data(key) : NULL,
-			key_data(newkey));
-	dev_dbg(dev, "key: %d %d update: %s\n",
+			key_data(newkey), master);
+	dev_dbg(dev, "key: %d %d update%s: %s\n",
 			key_serial(key), key_serial(newkey),
+			master ? "(master)" : "",
 			rc == 0 ? "success" : "fail");
 
 	nvdimm_put_key(newkey);
 	nvdimm_put_key(key);
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	if (master)
+		nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
+	else
+		nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
 	return rc;
 }
 
-int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
+int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
+		bool master)
 {
 	struct device *dev = &nvdimm->dev;
 	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
@@ -306,16 +312,24 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
 		return rc;
 	}
 
+	if (nvdimm->sec.ext_state !=
+			NVDIMM_SECURITY_MASTER_ENABLED	&& master) {
+		dev_warn(dev,
+			"Attempt to secure erase in wrong master state.\n");
+		return -EOPNOTSUPP;
+	}
+
 	key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
 	if (!key)
 		return -ENOKEY;
 
-	rc = nvdimm->sec.ops->erase(nvdimm, key_data(key));
-	dev_dbg(dev, "key: %d erase: %s\n", key_serial(key),
+	rc = nvdimm->sec.ops->erase(nvdimm, key_data(key), master);
+	dev_dbg(dev, "key: %d erase%s: %s\n", key_serial(key),
+			master ? "(master)" : "",
 			rc == 0 ? "success" : "fail");
 
 	nvdimm_put_key(key);
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
 	return rc;
 }
 
@@ -370,7 +384,7 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
 			rc == 0 ? "success" : "fail");
 
 	nvdimm_put_key(key);
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
 	nvdimm_set_security_busy(nvdimm);
 	return rc;
 }
@@ -412,5 +426,6 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
 		sysfs_notify_dirent(nvdimm->sec.overwrite_state);
 	nvdimm->sec.overwrite_tmo = 0;
 	nvdimm_clear_security_busy(nvdimm);
-	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
+	nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
 }
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index b728952ccb28..5383a22d6ba0 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -163,6 +163,8 @@ enum nvdimm_security_state {
 	NVDIMM_SECURITY_LOCKED,
 	NVDIMM_SECURITY_FROZEN,
 	NVDIMM_SECURITY_OVERWRITE,
+	NVDIMM_SECURITY_MASTER_ENABLED,
+	NVDIMM_SECURITY_MASTER_FROZEN,
 };
 
 #define NVDIMM_PASSPHRASE_LEN		32
@@ -173,17 +175,18 @@ struct nvdimm_key_data {
 };
 
 struct nvdimm_security_ops {
-	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
+	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm,
+			bool master);
 	int (*freeze)(struct nvdimm *nvdimm);
 	int (*change_key)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *old_data,
-			const struct nvdimm_key_data *new_data);
+			const struct nvdimm_key_data *new_data, bool master);
 	int (*unlock)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
 	int (*disable)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
 	int (*erase)(struct nvdimm *nvdimm,
-			const struct nvdimm_key_data *key_data);
+			const struct nvdimm_key_data *key_data, bool master);
 	int (*overwrite)(struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *key_data);
 	int (*query_overwrite)(struct nvdimm *nvdimm);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 14/17] tools/testing/nvdimm: Add test support for Intel nvdimm security DSMs
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (12 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 15/17] tools/testing/nvdimm: Add overwrite support for nfit_test Dave Jiang
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add nfit_test support for DSM functions "Get Security State",
"Set Passphrase", "Disable Passphrase", "Unlock Unit", "Freeze Lock",
and "Secure Erase" for the fake DIMMs.

Also adding a sysfs knob in order to put the DIMMs in "locked" state. The
order of testing DIMM unlocking would be.
1a. Disable DIMM X.
1b. Set Passphrase to DIMM X.
2. Write to
/sys/devices/platform/nfit_test.0/nfit_test_dimm/test_dimmX/lock_dimm
3. Renable DIMM X
4. Check DIMM X state via sysfs "security" attribute for nmemX.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/dimm_devs.c       |    2 
 tools/testing/nvdimm/Kbuild      |    1 
 tools/testing/nvdimm/dimm_devs.c |   41 +++++++++
 tools/testing/nvdimm/test/nfit.c |  180 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/nvdimm/dimm_devs.c

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 8825551aad84..4d8450285517 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -371,7 +371,7 @@ static ssize_t available_slots_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(available_slots);
 
-static ssize_t security_show(struct device *dev,
+__weak ssize_t security_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 33ea40777205..10ddf223055b 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -81,6 +81,7 @@ libnvdimm-$(CONFIG_BTT) += $(NVDIMM_SRC)/btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += $(NVDIMM_SRC)/pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += $(NVDIMM_SRC)/dax_devs.o
 libnvdimm-$(CONFIG_NVDIMM_KEYS) += $(NVDIMM_SRC)/security.o
+libnvdimm-y += dimm_devs.o
 libnvdimm-y += libnvdimm_test.o
 libnvdimm-y += config_check.o
 
diff --git a/tools/testing/nvdimm/dimm_devs.c b/tools/testing/nvdimm/dimm_devs.c
new file mode 100644
index 000000000000..e75238404555
--- /dev/null
+++ b/tools/testing/nvdimm/dimm_devs.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Intel Corp. 2018 */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/nd.h>
+#include "pmem.h"
+#include "pfn.h"
+#include "nd.h"
+#include "nd-core.h"
+
+ssize_t security_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvdimm *nvdimm = to_nvdimm(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, false);
+	nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
+
+	switch (nvdimm->sec.state) {
+	case NVDIMM_SECURITY_DISABLED:
+		return sprintf(buf, "disabled\n");
+	case NVDIMM_SECURITY_UNLOCKED:
+		return sprintf(buf, "unlocked\n");
+	case NVDIMM_SECURITY_LOCKED:
+		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;
+}
+
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 01ec04bf91b5..30f89fd740d9 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -142,6 +142,10 @@ static u32 handle[] = {
 
 static unsigned long dimm_fail_cmd_flags[ARRAY_SIZE(handle)];
 static int dimm_fail_cmd_code[ARRAY_SIZE(handle)];
+struct nfit_test_sec {
+	u8 state;
+	u8 passphrase[32];
+} dimm_sec_info[NUM_DCR];
 
 static const struct nd_intel_smart smart_def = {
 	.flags = ND_INTEL_SMART_HEALTH_VALID
@@ -933,6 +937,138 @@ static int override_return_code(int dimm, unsigned int func, int rc)
 	return rc;
 }
 
+static int nd_intel_test_cmd_security_status(struct nfit_test *t,
+		struct nd_intel_get_security_state *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	nd_cmd->status = 0;
+	nd_cmd->state = sec->state;
+	dev_dbg(dev, "security state (%#x) returned\n", nd_cmd->state);
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_unlock_unit(struct nfit_test *t,
+		struct nd_intel_unlock_unit *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_LOCKED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "unlock unit: invalid state: %#x\n",
+				sec->state);
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "unlock unit: invalid passphrase\n");
+	} else {
+		nd_cmd->status = 0;
+		sec->state = ND_INTEL_SEC_STATE_ENABLED;
+		dev_dbg(dev, "Unit unlocked\n");
+	}
+
+	dev_dbg(dev, "unlocking status returned: %#x\n", nd_cmd->status);
+	return 0;
+}
+
+static int nd_intel_test_cmd_set_pass(struct nfit_test *t,
+		struct nd_intel_set_passphrase *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (sec->state & ND_INTEL_SEC_STATE_FROZEN) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "set passphrase: wrong security state\n");
+	} else if (memcmp(nd_cmd->old_pass, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "set passphrase: wrong passphrase\n");
+	} else {
+		memcpy(sec->passphrase, nd_cmd->new_pass,
+				ND_INTEL_PASSPHRASE_SIZE);
+		sec->state |= ND_INTEL_SEC_STATE_ENABLED;
+		nd_cmd->status = 0;
+		dev_dbg(dev, "passphrase updated\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_freeze_lock(struct nfit_test *t,
+		struct nd_intel_freeze_lock *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "freeze lock: wrong security state\n");
+	} else {
+		sec->state |= ND_INTEL_SEC_STATE_FROZEN;
+		nd_cmd->status = 0;
+		dev_dbg(dev, "security frozen\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_disable_pass(struct nfit_test *t,
+		struct nd_intel_disable_passphrase *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "disable passphrase: wrong security state\n");
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "disable passphrase: wrong passphrase\n");
+	} else {
+		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		sec->state = 0;
+		dev_dbg(dev, "disable passphrase: done\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
+		struct nd_intel_secure_erase *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "secure erase: wrong security state\n");
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "secure erase: wrong passphrase\n");
+	} else {
+		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		sec->state = 0;
+		dev_dbg(dev, "secure erase: done\n");
+	}
+
+	return 0;
+}
+
 static int get_dimm(struct nfit_mem *nfit_mem, unsigned int func)
 {
 	int i;
@@ -980,6 +1116,30 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 				return i;
 
 			switch (func) {
+			case NVDIMM_INTEL_GET_SECURITY_STATE:
+				rc = nd_intel_test_cmd_security_status(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_UNLOCK_UNIT:
+				rc = nd_intel_test_cmd_unlock_unit(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_SET_PASSPHRASE:
+				rc = nd_intel_test_cmd_set_pass(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_DISABLE_PASSPHRASE:
+				rc = nd_intel_test_cmd_disable_pass(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_FREEZE_LOCK:
+				rc = nd_intel_test_cmd_freeze_lock(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_SECURE_ERASE:
+				rc = nd_intel_test_cmd_secure_erase(t,
+						buf, buf_len, i);
+				break;
 			case ND_INTEL_ENABLE_LSS_STATUS:
 				rc = nd_intel_test_cmd_set_lss_status(t,
 						buf, buf_len);
@@ -1313,10 +1473,22 @@ static ssize_t fail_cmd_code_store(struct device *dev, struct device_attribute *
 }
 static DEVICE_ATTR_RW(fail_cmd_code);
 
+static ssize_t lock_dimm_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	int dimm = dimm_name_to_id(dev);
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	sec->state = ND_INTEL_SEC_STATE_ENABLED | ND_INTEL_SEC_STATE_LOCKED;
+	return size;
+}
+static DEVICE_ATTR_WO(lock_dimm);
+
 static struct attribute *nfit_test_dimm_attributes[] = {
 	&dev_attr_fail_cmd.attr,
 	&dev_attr_fail_cmd_code.attr,
 	&dev_attr_handle.attr,
+	&dev_attr_lock_dimm.attr,
 	NULL,
 };
 
@@ -2195,6 +2367,14 @@ static void nfit_test0_setup(struct nfit_test *t)
 	set_bit(ND_INTEL_FW_FINISH_UPDATE, &acpi_desc->dimm_cmd_force_en);
 	set_bit(ND_INTEL_FW_FINISH_QUERY, &acpi_desc->dimm_cmd_force_en);
 	set_bit(ND_INTEL_ENABLE_LSS_STATUS, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_GET_SECURITY_STATE,
+			&acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_SET_PASSPHRASE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_DISABLE_PASSPHRASE,
+			&acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_UNLOCK_UNIT, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_FREEZE_LOCK, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_SECURE_ERASE, &acpi_desc->dimm_cmd_force_en);
 }
 
 static void nfit_test1_setup(struct nfit_test *t)

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 15/17] tools/testing/nvdimm: Add overwrite support for nfit_test
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (13 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 14/17] tools/testing/nvdimm: Add test support for Intel nvdimm security DSMs Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 16/17] tools/testing/nvdimm: add Intel DSM 1.8 " Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 17/17] libnvdimm/security: Add documentation for nvdimm security support Dave Jiang
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

With the implementation of Intel NVDIMM DSM overwrite, we are adding unit
test to nfit_test for testing of overwrite operation.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/nvdimm/test/nfit.c |   55 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 30f89fd740d9..3162fbf6e8a9 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -145,6 +145,7 @@ static int dimm_fail_cmd_code[ARRAY_SIZE(handle)];
 struct nfit_test_sec {
 	u8 state;
 	u8 passphrase[32];
+	u64 overwrite_end_time;
 } dimm_sec_info[NUM_DCR];
 
 static const struct nd_intel_smart smart_def = {
@@ -1069,6 +1070,50 @@ static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
 	return 0;
 }
 
+static int nd_intel_test_cmd_overwrite(struct nfit_test *t,
+		struct nd_intel_overwrite *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if ((sec->state & ND_INTEL_SEC_STATE_ENABLED) &&
+			memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "overwrite: wrong passphrase\n");
+		return 0;
+	}
+
+	memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+	sec->state = ND_INTEL_SEC_STATE_OVERWRITE;
+	dev_dbg(dev, "overwrite progressing.\n");
+	sec->overwrite_end_time = get_jiffies_64() + 5 * HZ;
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_query_overwrite(struct nfit_test *t,
+		struct nd_intel_query_overwrite *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_OVERWRITE)) {
+		nd_cmd->status = ND_INTEL_STATUS_OQUERY_SEQUENCE_ERR;
+		return 0;
+	}
+
+	if (time_is_before_jiffies64(sec->overwrite_end_time)) {
+		sec->overwrite_end_time = 0;
+		sec->state = 0;
+		dev_dbg(dev, "overwrite is complete\n");
+	} else
+		nd_cmd->status = ND_INTEL_STATUS_OQUERY_INPROGRESS;
+	return 0;
+}
+
 static int get_dimm(struct nfit_mem *nfit_mem, unsigned int func)
 {
 	int i;
@@ -1140,6 +1185,14 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 				rc = nd_intel_test_cmd_secure_erase(t,
 						buf, buf_len, i);
 				break;
+			case NVDIMM_INTEL_OVERWRITE:
+				rc = nd_intel_test_cmd_overwrite(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_QUERY_OVERWRITE:
+				rc = nd_intel_test_cmd_query_overwrite(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
 			case ND_INTEL_ENABLE_LSS_STATUS:
 				rc = nd_intel_test_cmd_set_lss_status(t,
 						buf, buf_len);
@@ -2375,6 +2428,8 @@ static void nfit_test0_setup(struct nfit_test *t)
 	set_bit(NVDIMM_INTEL_UNLOCK_UNIT, &acpi_desc->dimm_cmd_force_en);
 	set_bit(NVDIMM_INTEL_FREEZE_LOCK, &acpi_desc->dimm_cmd_force_en);
 	set_bit(NVDIMM_INTEL_SECURE_ERASE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_OVERWRITE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &acpi_desc->dimm_cmd_force_en);
 }
 
 static void nfit_test1_setup(struct nfit_test *t)

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 16/17] tools/testing/nvdimm: add Intel DSM 1.8 support for nfit_test
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (14 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 15/17] tools/testing/nvdimm: Add overwrite support for nfit_test Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  2018-12-11 20:26 ` [PATCH v13 17/17] libnvdimm/security: Add documentation for nvdimm security support Dave Jiang
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Adding test support for new Intel DSM from v1.8. The ability of simulating
master passphrase update and master secure erase have been added to
nfit_test.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/nvdimm/test/nfit.c |   86 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 3162fbf6e8a9..9a8d0e432e99 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -144,7 +144,9 @@ static unsigned long dimm_fail_cmd_flags[ARRAY_SIZE(handle)];
 static int dimm_fail_cmd_code[ARRAY_SIZE(handle)];
 struct nfit_test_sec {
 	u8 state;
+	u8 ext_state;
 	u8 passphrase[32];
+	u8 master_passphrase[32];
 	u64 overwrite_end_time;
 } dimm_sec_info[NUM_DCR];
 
@@ -947,6 +949,7 @@ static int nd_intel_test_cmd_security_status(struct nfit_test *t,
 
 	nd_cmd->status = 0;
 	nd_cmd->state = sec->state;
+	nd_cmd->extended_state = sec->ext_state;
 	dev_dbg(dev, "security state (%#x) returned\n", nd_cmd->state);
 
 	return 0;
@@ -1063,7 +1066,9 @@ static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
 		dev_dbg(dev, "secure erase: wrong passphrase\n");
 	} else {
 		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		memset(sec->master_passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
 		sec->state = 0;
+		sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
 		dev_dbg(dev, "secure erase: done\n");
 	}
 
@@ -1108,12 +1113,69 @@ static int nd_intel_test_cmd_query_overwrite(struct nfit_test *t,
 	if (time_is_before_jiffies64(sec->overwrite_end_time)) {
 		sec->overwrite_end_time = 0;
 		sec->state = 0;
+		sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
 		dev_dbg(dev, "overwrite is complete\n");
 	} else
 		nd_cmd->status = ND_INTEL_STATUS_OQUERY_INPROGRESS;
 	return 0;
 }
 
+static int nd_intel_test_cmd_master_set_pass(struct nfit_test *t,
+		struct nd_intel_set_master_passphrase *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->ext_state & ND_INTEL_SEC_ESTATE_ENABLED)) {
+		nd_cmd->status = ND_INTEL_STATUS_NOT_SUPPORTED;
+		dev_dbg(dev, "master set passphrase: in wrong state\n");
+	} else if (sec->ext_state & ND_INTEL_SEC_ESTATE_PLIMIT) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "master set passphrase: in wrong security state\n");
+	} else if (memcmp(nd_cmd->old_pass, sec->master_passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "master set passphrase: wrong passphrase\n");
+	} else {
+		memcpy(sec->master_passphrase, nd_cmd->new_pass,
+				ND_INTEL_PASSPHRASE_SIZE);
+		sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
+		dev_dbg(dev, "master passphrase: updated\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_master_secure_erase(struct nfit_test *t,
+		struct nd_intel_master_secure_erase *nd_cmd,
+		unsigned int buf_len, int dimm)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &dimm_sec_info[dimm];
+
+	if (!(sec->ext_state & ND_INTEL_SEC_ESTATE_ENABLED)) {
+		nd_cmd->status = ND_INTEL_STATUS_NOT_SUPPORTED;
+		dev_dbg(dev, "master secure erase: in wrong state\n");
+	} else if (sec->ext_state & ND_INTEL_SEC_ESTATE_PLIMIT) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "master secure erase: in wrong security state\n");
+	} else if (memcmp(nd_cmd->passphrase, sec->master_passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "master secure erase: wrong passphrase\n");
+	} else {
+		/* we do not erase master state passphrase ever */
+		sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
+		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		sec->state = 0;
+		dev_dbg(dev, "master secure erase: done\n");
+	}
+
+	return 0;
+}
+
+
 static int get_dimm(struct nfit_mem *nfit_mem, unsigned int func)
 {
 	int i;
@@ -1193,6 +1255,14 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 				rc = nd_intel_test_cmd_query_overwrite(t,
 						buf, buf_len, i - t->dcr_idx);
 				break;
+			case NVDIMM_INTEL_SET_MASTER_PASSPHRASE:
+				rc = nd_intel_test_cmd_master_set_pass(t,
+						buf, buf_len, i);
+				break;
+			case NVDIMM_INTEL_MASTER_SECURE_ERASE:
+				rc = nd_intel_test_cmd_master_secure_erase(t,
+						buf, buf_len, i);
+				break;
 			case ND_INTEL_ENABLE_LSS_STATUS:
 				rc = nd_intel_test_cmd_set_lss_status(t,
 						buf, buf_len);
@@ -1571,6 +1641,17 @@ static int nfit_test_dimm_init(struct nfit_test *t)
 	return 0;
 }
 
+static void security_init(struct nfit_test *t)
+{
+	int i;
+
+	for (i = 0; i < t->num_dcr; i++) {
+		struct nfit_test_sec *sec = &dimm_sec_info[i];
+
+		sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
+	}
+}
+
 static void smart_init(struct nfit_test *t)
 {
 	int i;
@@ -1649,6 +1730,7 @@ static int nfit_test0_alloc(struct nfit_test *t)
 	if (nfit_test_dimm_init(t))
 		return -ENOMEM;
 	smart_init(t);
+	security_init(t);
 	return ars_state_init(&t->pdev.dev, &t->ars_state);
 }
 
@@ -2430,6 +2512,10 @@ static void nfit_test0_setup(struct nfit_test *t)
 	set_bit(NVDIMM_INTEL_SECURE_ERASE, &acpi_desc->dimm_cmd_force_en);
 	set_bit(NVDIMM_INTEL_OVERWRITE, &acpi_desc->dimm_cmd_force_en);
 	set_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_SET_MASTER_PASSPHRASE,
+			&acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_MASTER_SECURE_ERASE,
+			&acpi_desc->dimm_cmd_force_en);
 }
 
 static void nfit_test1_setup(struct nfit_test *t)

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v13 17/17] libnvdimm/security: Add documentation for nvdimm security support
  2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
                   ` (15 preceding siblings ...)
  2018-12-11 20:26 ` [PATCH v13 16/17] tools/testing/nvdimm: add Intel DSM 1.8 " Dave Jiang
@ 2018-12-11 20:26 ` Dave Jiang
  16 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-11 20:26 UTC (permalink / raw)
  To: dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

Add theory of operation for the security support that's going into
libnvdimm.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/nvdimm/security.txt |  143 +++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 Documentation/nvdimm/security.txt

diff --git a/Documentation/nvdimm/security.txt b/Documentation/nvdimm/security.txt
new file mode 100644
index 000000000000..027160f9c3b8
--- /dev/null
+++ b/Documentation/nvdimm/security.txt
@@ -0,0 +1,143 @@
+NVDIMM SECURITY
+===============
+
+1. Introduction
+---------------
+
+With the introduction of Intel DSM v1.8 specification [1], security DSMs are
+introduced. The spec added the following security DSMs: "get security state",
+"set passphrase", "disable passphrase", "unlock unit", "freeze lock",
+"secure erase", and "overwrite". A security_ops data structure has been
+added to struct dimm in order to support the security operations and generic
+APIs are exposed to allow vendor neutral operations.
+
+2. Sysfs Interface
+------------------
+The "security" sysfs attribute is provided in the nvdimm sysfs directory. For
+example:
+/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/nmem0/security
+
+The "show" function of that attribute will display the security state for
+that DIMM. The following states are available: disabled, unlocked, locked,
+frozen, and overwrite. If security is not supported, the sysfs attribute
+will not be visible.
+
+The "store" function takes several commands when the attribute is written to
+in order to support some of the security functionalities:
+update <old_keyid> <new_keyid> - enable or update passphrase.
+disable <keyid> - disable enabled security and remove key.
+freeze - freeze changing of security states.
+erase <keyid> - generate new ecryption key for DIMM and crypto-scrambles
+	all existing user data using the user passphrase.
+overwrite <keyid> - wipe the entire nvdimm.
+master_update <keyid> <new_keyid> - enable or update master passphrase.
+master_erase <keyid> - generate new ecryption key for DIMM and crypto-scrambles
+	all existing user data using the master passphrase.
+master
+
+3. Key Management
+-----------------
+
+The key is associted to the payload by the DIMM id. For example:
+# cat /sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/nmem0/nfit/id
+8089-a2-1740-00000133
+The DIMM id would be provided along with the key payload (passphrase) to
+the kernel.
+
+The security keys are managed on the basis of a single key per DIMM. The
+key "passphrase" is expected to be 32bytes long. This is similar to the ATA
+security specification [2]. A key is initially acquired via the request_key()
+kernel API call during nvdimm unlock. It is up to the user to make sure that
+all the keys are in the kernel user keyring for unlock.
+
+A nvdimm encrypted-key of format enc32 has the description format of:
+nvdimm:<bus-provider-specific-unique-id>
+
+See file ``Documentation/security/keys/trusted-encrypted.rst`` for creating
+encrypted-keys of enc32 format. TPM usage with a master trusted key is
+preferred for sealing the encrypted-keys.
+
+4. Unlocking
+------------
+When the DIMMs are being enumerated by the kernel, the kernel will attempt to
+retrieve the key from the kernel user keyring. This is the only time
+a locked DIMM can be unlocked. Once unlocked, the DIMM will remain unlocked
+until reboot. Typically, an entity (i.e. shell script) will inject all the
+relevant encrypted-keys into the kernel user keyring during initramfs phase.
+It is also recommended that the keys are injected before libnvdimm is loaded
+by modprobe.
+
+5. Update
+---------
+When doing an update, it is expected that the existing key is removed from
+tkey key user keyring and injected as the old key. It's irrelevant what
+the key description is for the old key since we are only interested in the
+key id when doing the update operation. It is also expected that the new key
+is injected with the description format described from earlier in this
+document.  The update command written to the sysfs attribute will be with
+the format:
+update <old keyid> <new keyid>
+
+If there is no old keyid due to a security enabling, then a 0 should be
+passed in.
+
+6. Freeze
+---------
+The freeze operation does not require any keys. The security config can be
+frozen by a user with root privelege.
+
+7. Disable
+----------
+The security disable command format is:
+disable <keyid>
+
+An key with the current passphrase payload that is tied to the nvdimm should be
+in the kernel user keyring.
+
+8. Secure Erase
+---------------
+The command format for doing a secure erase is:
+erase <keyid>
+
+An key with the current passphrase payload that is tied to the nvdimm should be
+in the kernel user keyring.
+
+9. Overwrite
+------------
+The command format for doing an overwrite is:
+overwrite <keyid>
+
+Overwrite can be done without a key if security is not enabled. A key serial
+of 0 can be passed in to indicate no key.
+
+The sysfs attribute "security" can be polled to wait on overwrite completion.
+Overwrite can last tens of minutes or more depending on nvdimm size.
+
+An encrypted-key with the current user passphrase that is tied to the nvdimm
+should be injected and its keyid should be passed in via sysfs.
+
+10. Master Update
+-----------------
+The command format for doing a master update is:
+update <old keyid> <new keyid>
+
+The operating mechansim for master update is identical to update except the
+master passphrase key is passed to the kernel. The master passphrase key
+is just another encrypted-key.
+
+This command is only available when security is disabled.
+
+11. Master Erase
+----------------
+The command format for doing a master erase is:
+master_erase <current keyid>
+
+This command has the same operating mechanism as erase except the master
+passphrase key is passed to the kernel. The master passphrase key is just
+another encrypted-key.
+
+This command is only available when the master security is enabled, indicated
+by the extended security status.
+
+[1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf
+[2]: http://www.t13.org/documents/UploadedDocuments/docs2006/e05179r4-ACS-SecurityClarifications.pdf

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support
  2018-12-11 20:26 ` [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support Dave Jiang
@ 2018-12-11 23:30   ` Dan Williams
  0 siblings, 0 replies; 22+ messages in thread
From: Dan Williams @ 2018-12-11 23:30 UTC (permalink / raw)
  To: Dave Jiang; +Cc: David Howells, Mimi Zohar, linux-nvdimm

On Tue, Dec 11, 2018 at 12:26 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> With Intel DSM 1.8 [1] two new security DSMs are introduced. Enable/update
> master passphrase and master secure erase. The master passphrase allows
> a secure erase to be performed without the user passphrase that is set on
> the NVDIMM. The commands of master_update and master_erase are added to
> the sysfs knob in order to initiate the DSMs. They are similar in opeartion
> mechanism compare to update and erase.
>
> [1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/nfit/core.c   |    2 +
>  drivers/acpi/nfit/intel.c  |   61 ++++++++++++++++++++++++++++++++------------
>  drivers/nvdimm/dimm_devs.c |   33 +++++++++++++++++++++---
>  drivers/nvdimm/nd-core.h   |   10 ++++---
>  drivers/nvdimm/security.c  |   41 ++++++++++++++++++++----------
>  include/linux/libnvdimm.h  |    9 ++++--
>  6 files changed, 115 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 173517eb35b1..2e92b9d51c38 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -389,6 +389,8 @@ static u8 nfit_dsm_revid(unsigned family, unsigned func)
>                         [NVDIMM_INTEL_SECURE_ERASE] = 2,
>                         [NVDIMM_INTEL_OVERWRITE] = 2,
>                         [NVDIMM_INTEL_QUERY_OVERWRITE] = 2,
> +                       [NVDIMM_INTEL_SET_MASTER_PASSPHRASE] = 2,
> +                       [NVDIMM_INTEL_MASTER_SECURE_ERASE] = 2,
>                 },
>         };
>         u8 id;
> diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
> index 3c3f99dfc32c..09317ea23e72 100644
> --- a/drivers/acpi/nfit/intel.c
> +++ b/drivers/acpi/nfit/intel.c
> @@ -6,7 +6,8 @@
>  #include "intel.h"
>  #include "nfit.h"
>
> -static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
> +static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm,
> +               bool master)

Make this argument an enum and define values like NVDIMM_USER and
NVDIMM_MASTER, that way the code becomes more readable relative to
trying to recall what "true" and "false" mean. E.g.

    nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, NVDIMM_MASTER);

...also makes it easier to grep for all the places that care about the
master passphrase.

>  {
>         struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
>         struct {
> @@ -34,17 +35,28 @@ static enum nvdimm_security_state intel_security_state(struct nvdimm *nvdimm)
>                 return -EIO;
>
>         /* check and see if security is enabled and locked */
> -       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;
> +       if (master) {
> +               if (nd_cmd.cmd.extended_state & ND_INTEL_SEC_ESTATE_ENABLED)
> +                       return NVDIMM_SECURITY_MASTER_ENABLED;
> +               else if (nd_cmd.cmd.extended_state &
> +                               ND_INTEL_SEC_ESTATE_PLIMIT)
> +                       return NVDIMM_SECURITY_MASTER_FROZEN;

Do we need these states? If we're already passing in the "master" flag
then we can reuse the state names. NVDIMM_SECURITY_DISABLED and
NVDIMM_SECURITY_FROZEN. Because elsewhere we have code like:

    if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {

...where its not clear whether NVDIMM_SECURITY_FROZEN and
NVDIMM_SECURITY_OVERWRITE are the concern, or if the MASTER states are
also meant to be covered.


> +       } 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;
> +               }
>         }
> +
> +       /* this should cover master security disabled as well */
>         return NVDIMM_SECURITY_DISABLED;
>  }
>
> @@ -77,7 +89,8 @@ static int intel_security_freeze(struct nvdimm *nvdimm)
>
>  static int intel_security_change_key(struct nvdimm *nvdimm,
>                 const struct nvdimm_key_data *old_data,
> -               const struct nvdimm_key_data *new_data)
> +               const struct nvdimm_key_data *new_data,
> +               bool master)
>  {
>         struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
>         struct {
> @@ -85,7 +98,6 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
>                 struct nd_intel_set_passphrase cmd;
>         } nd_cmd = {
>                 .pkg = {
> -                       .nd_command = NVDIMM_INTEL_SET_PASSPHRASE,
>                         .nd_family = NVDIMM_FAMILY_INTEL,
>                         .nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2,
>                         .nd_size_out = ND_INTEL_STATUS_SIZE,
> @@ -94,7 +106,15 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
>         };
>         int rc;
>
> -       if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
> +       if (master)
> +               nd_cmd.pkg.nd_command = NVDIMM_INTEL_SET_MASTER_PASSPHRASE;
> +       else
> +               nd_cmd.pkg.nd_command = NVDIMM_INTEL_SET_PASSPHRASE;
> +
> +       if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask) ||
> +                       (master &&
> +                        !test_bit(NVDIMM_INTEL_SET_MASTER_PASSPHRASE,
> +                                &nfit_mem->dsm_mask)))

See below for some potential cleanups...

>                 return -ENOTTY;
>
>         if (old_data)
> @@ -206,7 +226,7 @@ static int intel_security_disable(struct nvdimm *nvdimm,
>  }
>
>  static int intel_security_erase(struct nvdimm *nvdimm,
> -               const struct nvdimm_key_data *key)
> +               const struct nvdimm_key_data *key, bool master)
>  {
>         int rc;
>         struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> @@ -219,14 +239,21 @@ static int intel_security_erase(struct nvdimm *nvdimm,
>                         .nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
>                         .nd_size_out = ND_INTEL_STATUS_SIZE,
>                         .nd_fw_size = ND_INTEL_STATUS_SIZE,
> -                       .nd_command = NVDIMM_INTEL_SECURE_ERASE,
>                 },
>                 .cmd = {
>                         .status = 0,
>                 },
>         };
>
> -       if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask))
> +       if (master)
> +               nd_cmd.pkg.nd_command = NVDIMM_INTEL_MASTER_SECURE_ERASE;
> +       else
> +               nd_cmd.pkg.nd_command = NVDIMM_INTEL_SECURE_ERASE;

Would be nice to keep to the c99 going. You could do:

    unsigned command = master ? NVDIMM_INTEL_MASTER_SECURE_ERASE :
NVDIMM_INTEL_SECURE_ERASE;

...before starting the struct declaration and then do:

    .nd_command = command,

...in the initialization.

> +
> +       if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask) ||
> +                       (master &&
> +                        !test_bit(NVDIMM_INTEL_MASTER_SECURE_ERASE,
> +                                &nfit_mem->dsm_mask)))

Similarly here you could do:

    test_bit(command, &nfit_mem->dsm_mask)

...and not worry about testing "master" again.

>                 return -ENOTTY;
>
>         /* flush all cache before we erase DIMM */
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index fa42774efb15..8825551aad84 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -387,6 +387,8 @@ static ssize_t security_show(struct device *dev,
>                 return sprintf(buf, "frozen\n");
>         case NVDIMM_SECURITY_OVERWRITE:
>                 return sprintf(buf, "overwrite\n");
> +       default:
> +               return -ENOTTY;
>         }
>
>         return -ENOTTY;
> @@ -428,7 +430,7 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
>                 rc = kstrtouint(nkeystr, 0, &newkey);
>                 if (rc < 0)
>                         return rc;
> -               rc = nvdimm_security_update(nvdimm, key, newkey);
> +               rc = nvdimm_security_update(nvdimm, key, newkey, false);
>                 if (rc < 0)
>                         return rc;
>         } else if (sysfs_streq(cmd, "erase")) {
> @@ -437,7 +439,7 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
>                 rc = kstrtouint(keystr, 0, &key);
>                 if (rc < 0)
>                         return rc;
> -               rc = nvdimm_security_erase(nvdimm, key);
> +               rc = nvdimm_security_erase(nvdimm, key, false);
>                 if (rc < 0)
>                         return rc;
>         } else if (sysfs_streq(cmd, "overwite")) {
> @@ -449,6 +451,27 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
>                 rc = nvdimm_security_overwrite(nvdimm, key);
>                 if (rc < 0)
>                         return rc;
> +       } else if (sysfs_streq(cmd, "master_update")) {
> +               if (rc != 3)
> +                       return -EINVAL;
> +               rc = kstrtouint(keystr, 0, &key);
> +               if (rc < 0)
> +                       return rc;
> +               rc = kstrtouint(nkeystr, 0, &newkey);
> +               if (rc < 0)
> +                       return rc;
> +               rc = nvdimm_security_update(nvdimm, key, newkey, true);
> +               if (rc < 0)
> +                       return rc;
> +       } else if (sysfs_streq(cmd, "master_erase")) {
> +               if (rc != 2)
> +                       return -EINVAL;
> +               rc = kstrtouint(keystr, 0, &key);
> +               if (rc < 0)
> +                       return rc;
> +               rc = nvdimm_security_erase(nvdimm, key, true);
> +               if (rc < 0)
> +                       return rc;
>         } else
>                 return -EINVAL;
>
> @@ -551,7 +574,9 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
>          * Security state must be initialized before device_add() for
>          * attribute visibility.
>          */
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       /* get security state and extended (master) state */
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
> +       nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
>         nd_device_register(dev);
>
>         return nvdimm;
> @@ -587,7 +612,7 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
>         }
>
>         rc = nvdimm->sec.ops->freeze(nvdimm);
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
>
>         return rc;
>  }
> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
> index 2c93e2139346..f3e05e32c530 100644
> --- a/drivers/nvdimm/nd-core.h
> +++ b/drivers/nvdimm/nd-core.h
> @@ -46,6 +46,7 @@ struct nvdimm {
>         struct {
>                 const struct nvdimm_security_ops *ops;
>                 enum nvdimm_security_state state;
> +               enum nvdimm_security_state ext_state;
>                 unsigned int overwrite_tmo;
>                 struct kernfs_node *overwrite_state;
>         } sec;
> @@ -53,18 +54,19 @@ struct nvdimm {
>  };
>
>  static inline enum nvdimm_security_state nvdimm_security_state(
> -               struct nvdimm *nvdimm)
> +               struct nvdimm *nvdimm, bool master)
>  {
>         if (!nvdimm->sec.ops)
>                 return -ENXIO;
>
> -       return nvdimm->sec.ops->state(nvdimm);
> +       return nvdimm->sec.ops->state(nvdimm, master);
>  }
>  int nvdimm_security_freeze(struct nvdimm *nvdimm);
>  int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid);
>  int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
> -               unsigned int new_keyid);
> -int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid);
> +               unsigned int new_keyid, bool master);
> +int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
> +               bool master);
>  static inline int nvdimm_security_check_busy(struct nvdimm *nvdimm)
>  {
>         if (test_bit(NDD_SECURITY_BUSY, &nvdimm->flags))
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index 3603c5fda1cf..a10eaf4e23c3 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -121,7 +121,8 @@ static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
>          * Send the same key to the hardware as new and old key to
>          * verify that the key is good.
>          */
> -       rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key), key_data(key));
> +       rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key),
> +                       key_data(key), false);
>         if (rc < 0) {
>                 nvdimm_put_key(key);
>                 key = NULL;
> @@ -174,7 +175,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->sec.state = nvdimm_security_state(nvdimm, false);
>         return rc;
>  }
>
> @@ -224,12 +225,12 @@ 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->sec.state = nvdimm_security_state(nvdimm, false);
>         return rc;
>  }
>
>  int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
> -               unsigned int new_keyid)
> +               unsigned int new_keyid, bool master)
>  {
>         struct device *dev = &nvdimm->dev;
>         struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
> @@ -264,18 +265,23 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
>         }
>
>         rc = nvdimm->sec.ops->change_key(nvdimm, key ? key_data(key) : NULL,
> -                       key_data(newkey));
> -       dev_dbg(dev, "key: %d %d update: %s\n",
> +                       key_data(newkey), master);
> +       dev_dbg(dev, "key: %d %d update%s: %s\n",
>                         key_serial(key), key_serial(newkey),
> +                       master ? "(master)" : "",
>                         rc == 0 ? "success" : "fail");
>
>         nvdimm_put_key(newkey);
>         nvdimm_put_key(key);
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       if (master)
> +               nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
> +       else
> +               nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
>         return rc;
>  }
>
> -int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
> +int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
> +               bool master)
>  {
>         struct device *dev = &nvdimm->dev;
>         struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
> @@ -306,16 +312,24 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
>                 return rc;
>         }
>
> +       if (nvdimm->sec.ext_state !=
> +                       NVDIMM_SECURITY_MASTER_ENABLED  && master) {
> +               dev_warn(dev,
> +                       "Attempt to secure erase in wrong master state.\n");
> +               return -EOPNOTSUPP;
> +       }
> +
>         key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
>         if (!key)
>                 return -ENOKEY;
>
> -       rc = nvdimm->sec.ops->erase(nvdimm, key_data(key));
> -       dev_dbg(dev, "key: %d erase: %s\n", key_serial(key),
> +       rc = nvdimm->sec.ops->erase(nvdimm, key_data(key), master);
> +       dev_dbg(dev, "key: %d erase%s: %s\n", key_serial(key),
> +                       master ? "(master)" : "",
>                         rc == 0 ? "success" : "fail");
>
>         nvdimm_put_key(key);
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
>         return rc;
>  }
>
> @@ -370,7 +384,7 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
>                         rc == 0 ? "success" : "fail");
>
>         nvdimm_put_key(key);
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
>         nvdimm_set_security_busy(nvdimm);
>         return rc;
>  }
> @@ -412,5 +426,6 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
>                 sysfs_notify_dirent(nvdimm->sec.overwrite_state);
>         nvdimm->sec.overwrite_tmo = 0;
>         nvdimm_clear_security_busy(nvdimm);
> -       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm, false);
> +       nvdimm->sec.ext_state = nvdimm_security_state(nvdimm, true);
>  }
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index b728952ccb28..5383a22d6ba0 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -163,6 +163,8 @@ enum nvdimm_security_state {
>         NVDIMM_SECURITY_LOCKED,
>         NVDIMM_SECURITY_FROZEN,
>         NVDIMM_SECURITY_OVERWRITE,
> +       NVDIMM_SECURITY_MASTER_ENABLED,
> +       NVDIMM_SECURITY_MASTER_FROZEN,

As per above I think we can get by without these.

>  };
>
>  #define NVDIMM_PASSPHRASE_LEN          32
> @@ -173,17 +175,18 @@ struct nvdimm_key_data {
>  };
>
>  struct nvdimm_security_ops {
> -       enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
> +       enum nvdimm_security_state (*state)(struct nvdimm *nvdimm,
> +                       bool master);
>         int (*freeze)(struct nvdimm *nvdimm);
>         int (*change_key)(struct nvdimm *nvdimm,
>                         const struct nvdimm_key_data *old_data,
> -                       const struct nvdimm_key_data *new_data);
> +                       const struct nvdimm_key_data *new_data, bool master);
>         int (*unlock)(struct nvdimm *nvdimm,
>                         const struct nvdimm_key_data *key_data);
>         int (*disable)(struct nvdimm *nvdimm,
>                         const struct nvdimm_key_data *key_data);
>         int (*erase)(struct nvdimm *nvdimm,
> -                       const struct nvdimm_key_data *key_data);
> +                       const struct nvdimm_key_data *key_data, bool master);
>         int (*overwrite)(struct nvdimm *nvdimm,
>                         const struct nvdimm_key_data *key_data);
>         int (*query_overwrite)(struct nvdimm *nvdimm);
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support
  2018-12-11 20:26 ` [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support Dave Jiang
@ 2018-12-11 23:44   ` Dan Williams
  2018-12-12  0:33     ` Dave Jiang
  0 siblings, 1 reply; 22+ messages in thread
From: Dan Williams @ 2018-12-11 23:44 UTC (permalink / raw)
  To: Dave Jiang; +Cc: David Howells, Mimi Zohar, linux-nvdimm

On Tue, Dec 11, 2018 at 12:26 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> We are adding support for the security calls of ovewrite and query
> overwrite introduced from Intel DSM spec v1.7. This will allow triggering
> of overwrite on Intel NVDIMMs. The overwrite operation can take tens
> of minutes. When the overwrite DSM is issued successfully, the NVDIMMs
> will be unaccessible. The kernel will do backoff polling to detect when
> the overwrite process is completed. According to the DSM spec v1.7,
> the 128G NVDIMMs can take up to 15mins to perform overwrite and larger
> DIMMs will take longer.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/nfit/core.c   |    5 ++
>  drivers/acpi/nfit/intel.c  |   93 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/nfit/nfit.h   |    1
>  drivers/nvdimm/core.c      |    3 +
>  drivers/nvdimm/dimm_devs.c |   35 +++++++++++++++-
>  drivers/nvdimm/nd-core.h   |    8 ++++
>  drivers/nvdimm/security.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/libnvdimm.h  |    4 ++
>  8 files changed, 245 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 77f188cd8023..173517eb35b1 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2043,6 +2043,11 @@ 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/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
> index ce65111ea4e1..3c3f99dfc32c 100644
> --- a/drivers/acpi/nfit/intel.c
> +++ b/drivers/acpi/nfit/intel.c
> @@ -254,6 +254,97 @@ static int intel_security_erase(struct nvdimm *nvdimm,
>         return 0;
>  }
>
> +static int intel_security_query_overwrite(struct nvdimm *nvdimm)
> +{
> +       int rc;
> +       struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> +       struct {
> +               struct nd_cmd_pkg pkg;
> +               struct nd_intel_query_overwrite cmd;
> +       } nd_cmd = {
> +               .pkg = {
> +                       .nd_command = NVDIMM_INTEL_QUERY_OVERWRITE,
> +                       .nd_family = NVDIMM_FAMILY_INTEL,
> +                       .nd_size_in = 0,
> +                       .nd_size_out = ND_INTEL_STATUS_SIZE,
> +                       .nd_fw_size = ND_INTEL_STATUS_SIZE,
> +               },
> +               .cmd = {
> +                       .status = 0,
> +               },
> +       };
> +
> +       if (!test_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &nfit_mem->dsm_mask))
> +               return -ENOTTY;
> +
> +       rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
> +       if (rc < 0)
> +               return rc;
> +
> +       switch (nd_cmd.cmd.status) {
> +       case 0:
> +               break;
> +       case ND_INTEL_STATUS_OQUERY_INPROGRESS:
> +               return -EBUSY;
> +       default:
> +               return -ENXIO;
> +       }
> +
> +       /* flush all cache before we make the nvdimms available */
> +       nvdimm_invalidate_cache();
> +       nfit_mem->overwrite = false;
> +       return 0;
> +}
> +
> +static int intel_security_overwrite(struct nvdimm *nvdimm,
> +               const struct nvdimm_key_data *nkey)
> +{
> +       int rc;
> +       struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> +       struct {
> +               struct nd_cmd_pkg pkg;
> +               struct nd_intel_overwrite cmd;
> +       } nd_cmd = {
> +               .pkg = {
> +                       .nd_command = NVDIMM_INTEL_OVERWRITE,
> +                       .nd_family = NVDIMM_FAMILY_INTEL,
> +                       .nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
> +                       .nd_size_out = ND_INTEL_STATUS_SIZE,
> +                       .nd_fw_size = ND_INTEL_STATUS_SIZE,
> +               },
> +               .cmd = {
> +                       .status = 0,
> +               },
> +       };
> +
> +       if (!test_bit(NVDIMM_INTEL_OVERWRITE, &nfit_mem->dsm_mask))
> +               return -ENOTTY;
> +
> +       /* flush all cache before we erase DIMM */
> +       nvdimm_invalidate_cache();
> +       if (nkey)
> +               memcpy(nd_cmd.cmd.passphrase, nkey->data,
> +                               sizeof(nd_cmd.cmd.passphrase));
> +       rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
> +       if (rc < 0)
> +               return rc;
> +
> +       switch (nd_cmd.cmd.status) {
> +       case 0:
> +               nfit_mem->overwrite = true;
> +               break;
> +       case ND_INTEL_STATUS_OVERWRITE_UNSUPPORTED:
> +               return -ENOTSUPP;
> +       case ND_INTEL_STATUS_INVALID_PASS:
> +               return -EINVAL;
> +       case ND_INTEL_STATUS_INVALID_STATE:
> +       default:
> +               return -ENXIO;
> +       }
> +
> +       return 0;
> +}
> +
>  /*
>   * TODO: define a cross arch wbinvd equivalent when/if
>   * NVDIMM_FAMILY_INTEL command support arrives on another arch.
> @@ -278,6 +369,8 @@ static const struct nvdimm_security_ops __intel_security_ops = {
>  #ifdef CONFIG_X86
>         .unlock = intel_security_unlock,
>         .erase = intel_security_erase,
> +       .overwrite = intel_security_overwrite,
> +       .query_overwrite = intel_security_query_overwrite,
>  #endif
>  };
>
> diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
> index 33691aecfcee..e0ee54049c89 100644
> --- a/drivers/acpi/nfit/nfit.h
> +++ b/drivers/acpi/nfit/nfit.h
> @@ -208,6 +208,7 @@ struct nfit_mem {
>         unsigned long flags;
>         u32 dirty_shutdown;
>         int family;
> +       bool overwrite;

What is this for? It's not used in this patch.

>  };
>
>  struct acpi_nfit_desc {
> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
> index acce050856a8..b2496c06178b 100644
> --- a/drivers/nvdimm/core.c
> +++ b/drivers/nvdimm/core.c
> @@ -437,6 +437,9 @@ static __init int libnvdimm_init(void)
>  {
>         int rc;
>
> +       rc = nvdimm_devs_init();
> +       if (rc)
> +               return rc;
>         rc = nvdimm_bus_init();
>         if (rc)
>                 return rc;
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index bfdc4824ba11..fa42774efb15 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -24,6 +24,7 @@
>  #include "nd.h"
>
>  static DEFINE_IDA(dimm_ida);
> +struct workqueue_struct *nvdimm_wq;

static?

Or why not use the system workqueue? I don't think this necessarily
needs its own workqueue.

>
>  /*
>   * Retrieve bus and dimm handle and return if this bus supports
> @@ -439,6 +440,15 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
>                 rc = nvdimm_security_erase(nvdimm, key);
>                 if (rc < 0)
>                         return rc;
> +       } else if (sysfs_streq(cmd, "overwite")) {

"overwrite"

> +               if (rc != 2)
> +                       return -EINVAL;
> +               rc = kstrtouint(keystr, 0, &key);
> +               if (rc < 0)
> +                       return rc;
> +               rc = nvdimm_security_overwrite(nvdimm, key);
> +               if (rc < 0)
> +                       return rc;
>         } else
>                 return -EINVAL;
>
> @@ -491,7 +501,8 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
>         /* 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->erase
> +                       || nvdimm->sec.ops->overwrite)
>                 return a->mode;
>         return 0444;
>  }
> @@ -534,6 +545,8 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
>         dev->devt = MKDEV(nvdimm_major, nvdimm->id);
>         dev->groups = groups;
>         nvdimm->sec.ops = sec_ops;
> +       nvdimm->sec.overwrite_tmo = 0;
> +       INIT_DELAYED_WORK(&nvdimm->dwork, nvdimm_security_overwrite_query);
>         /*
>          * Security state must be initialized before device_add() for
>          * attribute visibility.
> @@ -545,6 +558,16 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
>  }
>  EXPORT_SYMBOL_GPL(__nvdimm_create);
>
> +int nvdimm_security_setup_events(struct nvdimm *nvdimm)
> +{
> +       nvdimm->sec.overwrite_state = sysfs_get_dirent(nvdimm->dev.kobj.sd,
> +                       "security");
> +       if (!nvdimm->sec.overwrite_state)
> +               return -ENODEV;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(nvdimm_security_setup_events);
> +
>  int nvdimm_security_freeze(struct nvdimm *nvdimm)
>  {
>         int rc;
> @@ -839,7 +862,17 @@ int nvdimm_bus_check_dimm_count(struct nvdimm_bus *nvdimm_bus, int dimm_count)
>  }
>  EXPORT_SYMBOL_GPL(nvdimm_bus_check_dimm_count);
>
> +int __init nvdimm_devs_init(void)
> +{
> +       nvdimm_wq = create_singlethread_workqueue("nvdimm");
> +       if (!nvdimm_wq)
> +               return -ENOMEM;
> +
> +       return 0;
> +}
> +
>  void __exit nvdimm_devs_exit(void)
>  {
> +       destroy_workqueue(nvdimm_wq);

I don't see a call to cancel_delayed_work_sync(), what ensures that
the dimm is not hot removed while overwrite is in flight? It's
probably ok to not wait for completion because the driver will
reacquire the state at the next driver load, but the work needs to be
cancelled, shutoff, and flushed before the device_del() of the nvdimm
device.



>         ida_destroy(&dimm_ida);
>  }
> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
> index 353f945cda5b..2c93e2139346 100644
> --- a/drivers/nvdimm/nd-core.h
> +++ b/drivers/nvdimm/nd-core.h
> @@ -21,6 +21,7 @@
>  extern struct list_head nvdimm_bus_list;
>  extern struct mutex nvdimm_bus_list_mutex;
>  extern int nvdimm_major;
> +extern struct workqueue_struct *nvdimm_wq;
>
>  struct nvdimm_bus {
>         struct nvdimm_bus_descriptor *nd_desc;
> @@ -45,7 +46,10 @@ struct nvdimm {
>         struct {
>                 const struct nvdimm_security_ops *ops;
>                 enum nvdimm_security_state state;
> +               unsigned int overwrite_tmo;
> +               struct kernfs_node *overwrite_state;
>         } sec;
> +       struct delayed_work dwork;
>  };
>
>  static inline enum nvdimm_security_state nvdimm_security_state(
> @@ -78,6 +82,9 @@ static inline void nvdimm_clear_security_busy(struct nvdimm *nvdimm)
>         clear_bit(NDD_SECURITY_BUSY, &nvdimm->flags);
>  }
>
> +int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid);
> +void nvdimm_security_overwrite_query(struct work_struct *work);
> +
>  /**
>   * struct blk_alloc_info - tracking info for BLK dpa scanning
>   * @nd_mapping: blk region mapping boundaries
> @@ -110,6 +117,7 @@ static inline bool is_memory(struct device *dev)
>  struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev);
>  int __init nvdimm_bus_init(void);
>  void nvdimm_bus_exit(void);
> +int nvdimm_devs_init(void);
>  void nvdimm_devs_exit(void);
>  void nd_region_devs_exit(void);
>  void nd_region_probe_success(struct nvdimm_bus *nvdimm_bus, struct device *dev);
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index 5a03dffd0056..3603c5fda1cf 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -282,7 +282,7 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
>         struct key *key;
>         int rc;
>
> -       /* The bus lock should be held at the top level of the call stack */
> +       /* the bus lock should be held at the top level of the call stack */

Unintended change? Capitalized sentences is the local nvdimm custom.

>         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>
>         if (!nvdimm->sec.ops || !nvdimm->sec.ops->erase
> @@ -318,3 +318,99 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
>         nvdimm->sec.state = nvdimm_security_state(nvdimm);
>         return rc;
>  }
> +
> +int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
> +{
> +       struct device *dev = &nvdimm->dev;
> +       struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
> +       struct key *key;
> +       int rc;
> +
> +       /* the bus lock should be held at the top level of the call stack */
> +       lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
> +
> +       if (!nvdimm->sec.ops || !nvdimm->sec.ops->overwrite
> +                       || nvdimm->sec.state < 0)
> +               return -EIO;
> +
> +       if (atomic_read(&nvdimm->busy)) {
> +               dev_warn(dev, "Unable to overwrite while DIMM active.\n");
> +               nvdimm_clear_security_busy(nvdimm);
> +               return -EBUSY;
> +       }
> +
> +       if (dev_get_drvdata(dev)) {

Use "dev->driver != NULL" as the "is enabled" test. dev_get_drvdata()
works today, but I've seen driver-core patches that tried to make
assumptions about the availability of drvdata for the core to use
while dev->driver is NULL.


> +               dev_warn(dev, "Unable to overwrite while DIMM active.\n");
> +               nvdimm_clear_security_busy(nvdimm);
> +               return -EINVAL;
> +       }
> +
> +       if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
> +               dev_warn(dev, "Incorrect security state: %d\n",
> +                               nvdimm->sec.state);
> +               return -EIO;
> +       }
> +
> +       rc = nvdimm_security_check_busy(nvdimm);
> +       if (rc < 0) {
> +               dev_warn(dev, "Security operation in progress.\n");
> +               return rc;
> +       }
> +
> +       if (keyid == 0)
> +               key = NULL;
> +       else {
> +               key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
> +               if (!key)
> +                       return -ENOKEY;
> +       }
> +
> +       rc = nvdimm->sec.ops->overwrite(nvdimm, key ? key_data(key) : NULL);
> +       dev_dbg(dev, "key: %d overwrite submission: %s\n", key_serial(key),
> +                       rc == 0 ? "success" : "fail");
> +
> +       nvdimm_put_key(key);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +       nvdimm_set_security_busy(nvdimm);
> +       return rc;
> +}
> +
> +void nvdimm_security_overwrite_query(struct work_struct *work)
> +{
> +       struct nvdimm_bus *nvdimm_bus;
> +       struct nvdimm *nvdimm;
> +       int rc;
> +       unsigned int tmo;
> +
> +       nvdimm = container_of(work, typeof(*nvdimm), dwork.work);
> +       nvdimm_bus = walk_to_nvdimm_bus(&nvdimm->dev);
> +       tmo = nvdimm->sec.overwrite_tmo;
> +
> +       /* The bus lock should be held at the top level of the call stack */
> +       lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
> +

The reconfig_mutex should have been dropped after the work was queued,
so we would need to reacquire and revalidate state here.

We'd also need to abort and not re-queue if the nvdimm device is going away.

> +       if (!nvdimm->sec.ops || !nvdimm->sec.ops->query_overwrite
> +                       || nvdimm->sec.state < 0)
> +               return;
> +
> +       rc = nvdimm->sec.ops->query_overwrite(nvdimm);
> +       if (rc == -EBUSY) {
> +
> +               /* setup delayed work again */
> +               tmo += 10;
> +               queue_delayed_work(nvdimm_wq, &nvdimm->dwork, tmo * HZ);
> +               nvdimm->sec.overwrite_tmo = min(15U * 60U, tmo);
> +               return;
> +       }
> +
> +       if (rc < 0)
> +               dev_warn(&nvdimm->dev, "overwrite failed\n");
> +       else
> +               dev_info(&nvdimm->dev, "overwrite completed\n");

Let's do dev_dbg(), no need to be chatty on success, and the driver is
already notifying userspace of the completion event.

> +
> +       if (nvdimm->sec.overwrite_state)
> +               sysfs_notify_dirent(nvdimm->sec.overwrite_state);
> +       nvdimm->sec.overwrite_tmo = 0;
> +       nvdimm_clear_security_busy(nvdimm);
> +       nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +}
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 8507d2896ae0..b728952ccb28 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -184,6 +184,9 @@ struct nvdimm_security_ops {
>                         const struct nvdimm_key_data *key_data);
>         int (*erase)(struct nvdimm *nvdimm,
>                         const struct nvdimm_key_data *key_data);
> +       int (*overwrite)(struct nvdimm *nvdimm,
> +                       const struct nvdimm_key_data *key_data);
> +       int (*query_overwrite)(struct nvdimm *nvdimm);
>  };
>
>  void badrange_init(struct badrange *badrange);
> @@ -221,6 +224,7 @@ 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,
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support
  2018-12-11 23:44   ` Dan Williams
@ 2018-12-12  0:33     ` Dave Jiang
  0 siblings, 0 replies; 22+ messages in thread
From: Dave Jiang @ 2018-12-12  0:33 UTC (permalink / raw)
  To: Dan Williams; +Cc: David Howells, Mimi Zohar, linux-nvdimm



On 12/11/18 4:44 PM, Dan Williams wrote:
> On Tue, Dec 11, 2018 at 12:26 PM Dave Jiang <dave.jiang@intel.com> wrote:
>>
>> We are adding support for the security calls of ovewrite and query
>> overwrite introduced from Intel DSM spec v1.7. This will allow triggering
>> of overwrite on Intel NVDIMMs. The overwrite operation can take tens
>> of minutes. When the overwrite DSM is issued successfully, the NVDIMMs
>> will be unaccessible. The kernel will do backoff polling to detect when
>> the overwrite process is completed. According to the DSM spec v1.7,
>> the 128G NVDIMMs can take up to 15mins to perform overwrite and larger
>> DIMMs will take longer.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/acpi/nfit/core.c   |    5 ++
>>  drivers/acpi/nfit/intel.c  |   93 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/acpi/nfit/nfit.h   |    1
>>  drivers/nvdimm/core.c      |    3 +
>>  drivers/nvdimm/dimm_devs.c |   35 +++++++++++++++-
>>  drivers/nvdimm/nd-core.h   |    8 ++++
>>  drivers/nvdimm/security.c  |   98 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/libnvdimm.h  |    4 ++
>>  8 files changed, 245 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 77f188cd8023..173517eb35b1 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -2043,6 +2043,11 @@ 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/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
>> index ce65111ea4e1..3c3f99dfc32c 100644
>> --- a/drivers/acpi/nfit/intel.c
>> +++ b/drivers/acpi/nfit/intel.c
>> @@ -254,6 +254,97 @@ static int intel_security_erase(struct nvdimm *nvdimm,
>>         return 0;
>>  }
>>
>> +static int intel_security_query_overwrite(struct nvdimm *nvdimm)
>> +{
>> +       int rc;
>> +       struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
>> +       struct {
>> +               struct nd_cmd_pkg pkg;
>> +               struct nd_intel_query_overwrite cmd;
>> +       } nd_cmd = {
>> +               .pkg = {
>> +                       .nd_command = NVDIMM_INTEL_QUERY_OVERWRITE,
>> +                       .nd_family = NVDIMM_FAMILY_INTEL,
>> +                       .nd_size_in = 0,
>> +                       .nd_size_out = ND_INTEL_STATUS_SIZE,
>> +                       .nd_fw_size = ND_INTEL_STATUS_SIZE,
>> +               },
>> +               .cmd = {
>> +                       .status = 0,
>> +               },
>> +       };
>> +
>> +       if (!test_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &nfit_mem->dsm_mask))
>> +               return -ENOTTY;
>> +
>> +       rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
>> +       if (rc < 0)
>> +               return rc;
>> +
>> +       switch (nd_cmd.cmd.status) {
>> +       case 0:
>> +               break;
>> +       case ND_INTEL_STATUS_OQUERY_INPROGRESS:
>> +               return -EBUSY;
>> +       default:
>> +               return -ENXIO;
>> +       }
>> +
>> +       /* flush all cache before we make the nvdimms available */
>> +       nvdimm_invalidate_cache();
>> +       nfit_mem->overwrite = false;
>> +       return 0;
>> +}
>> +
>> +static int intel_security_overwrite(struct nvdimm *nvdimm,
>> +               const struct nvdimm_key_data *nkey)
>> +{
>> +       int rc;
>> +       struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
>> +       struct {
>> +               struct nd_cmd_pkg pkg;
>> +               struct nd_intel_overwrite cmd;
>> +       } nd_cmd = {
>> +               .pkg = {
>> +                       .nd_command = NVDIMM_INTEL_OVERWRITE,
>> +                       .nd_family = NVDIMM_FAMILY_INTEL,
>> +                       .nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
>> +                       .nd_size_out = ND_INTEL_STATUS_SIZE,
>> +                       .nd_fw_size = ND_INTEL_STATUS_SIZE,
>> +               },
>> +               .cmd = {
>> +                       .status = 0,
>> +               },
>> +       };
>> +
>> +       if (!test_bit(NVDIMM_INTEL_OVERWRITE, &nfit_mem->dsm_mask))
>> +               return -ENOTTY;
>> +
>> +       /* flush all cache before we erase DIMM */
>> +       nvdimm_invalidate_cache();
>> +       if (nkey)
>> +               memcpy(nd_cmd.cmd.passphrase, nkey->data,
>> +                               sizeof(nd_cmd.cmd.passphrase));
>> +       rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
>> +       if (rc < 0)
>> +               return rc;
>> +
>> +       switch (nd_cmd.cmd.status) {
>> +       case 0:
>> +               nfit_mem->overwrite = true;
>> +               break;
>> +       case ND_INTEL_STATUS_OVERWRITE_UNSUPPORTED:
>> +               return -ENOTSUPP;
>> +       case ND_INTEL_STATUS_INVALID_PASS:
>> +               return -EINVAL;
>> +       case ND_INTEL_STATUS_INVALID_STATE:
>> +       default:
>> +               return -ENXIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  /*
>>   * TODO: define a cross arch wbinvd equivalent when/if
>>   * NVDIMM_FAMILY_INTEL command support arrives on another arch.
>> @@ -278,6 +369,8 @@ static const struct nvdimm_security_ops __intel_security_ops = {
>>  #ifdef CONFIG_X86
>>         .unlock = intel_security_unlock,
>>         .erase = intel_security_erase,
>> +       .overwrite = intel_security_overwrite,
>> +       .query_overwrite = intel_security_query_overwrite,
>>  #endif
>>  };
>>
>> diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
>> index 33691aecfcee..e0ee54049c89 100644
>> --- a/drivers/acpi/nfit/nfit.h
>> +++ b/drivers/acpi/nfit/nfit.h
>> @@ -208,6 +208,7 @@ struct nfit_mem {
>>         unsigned long flags;
>>         u32 dirty_shutdown;
>>         int family;
>> +       bool overwrite;
> 
> What is this for? It's not used in this patch.

I'm missing couple lines in intel_dimm_security_state() to short circuit
things and set NVDIMM_SECURITY_OVERWRITE when this is set when I was
moving things to the squashed tree.

> 
>>  };
>>
>>  struct acpi_nfit_desc {
>> diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
>> index acce050856a8..b2496c06178b 100644
>> --- a/drivers/nvdimm/core.c
>> +++ b/drivers/nvdimm/core.c
>> @@ -437,6 +437,9 @@ static __init int libnvdimm_init(void)
>>  {
>>         int rc;
>>
>> +       rc = nvdimm_devs_init();
>> +       if (rc)
>> +               return rc;
>>         rc = nvdimm_bus_init();
>>         if (rc)
>>                 return rc;
>> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
>> index bfdc4824ba11..fa42774efb15 100644
>> --- a/drivers/nvdimm/dimm_devs.c
>> +++ b/drivers/nvdimm/dimm_devs.c
>> @@ -24,6 +24,7 @@
>>  #include "nd.h"
>>
>>  static DEFINE_IDA(dimm_ida);
>> +struct workqueue_struct *nvdimm_wq;
> 
> static?
> 
> Or why not use the system workqueue? I don't think this necessarily
> needs its own workqueue.
> 
>>
>>  /*
>>   * Retrieve bus and dimm handle and return if this bus supports
>> @@ -439,6 +440,15 @@ static ssize_t __security_store(struct device *dev, const char *buf, size_t len)
>>                 rc = nvdimm_security_erase(nvdimm, key);
>>                 if (rc < 0)
>>                         return rc;
>> +       } else if (sysfs_streq(cmd, "overwite")) {
> 
> "overwrite"
> 
>> +               if (rc != 2)
>> +                       return -EINVAL;
>> +               rc = kstrtouint(keystr, 0, &key);
>> +               if (rc < 0)
>> +                       return rc;
>> +               rc = nvdimm_security_overwrite(nvdimm, key);
>> +               if (rc < 0)
>> +                       return rc;
>>         } else
>>                 return -EINVAL;
>>
>> @@ -491,7 +501,8 @@ static umode_t nvdimm_visible(struct kobject *kobj, struct attribute *a, int n)
>>         /* 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->erase
>> +                       || nvdimm->sec.ops->overwrite)
>>                 return a->mode;
>>         return 0444;
>>  }
>> @@ -534,6 +545,8 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
>>         dev->devt = MKDEV(nvdimm_major, nvdimm->id);
>>         dev->groups = groups;
>>         nvdimm->sec.ops = sec_ops;
>> +       nvdimm->sec.overwrite_tmo = 0;
>> +       INIT_DELAYED_WORK(&nvdimm->dwork, nvdimm_security_overwrite_query);
>>         /*
>>          * Security state must be initialized before device_add() for
>>          * attribute visibility.
>> @@ -545,6 +558,16 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus,
>>  }
>>  EXPORT_SYMBOL_GPL(__nvdimm_create);
>>
>> +int nvdimm_security_setup_events(struct nvdimm *nvdimm)
>> +{
>> +       nvdimm->sec.overwrite_state = sysfs_get_dirent(nvdimm->dev.kobj.sd,
>> +                       "security");
>> +       if (!nvdimm->sec.overwrite_state)
>> +               return -ENODEV;
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvdimm_security_setup_events);
>> +
>>  int nvdimm_security_freeze(struct nvdimm *nvdimm)
>>  {
>>         int rc;
>> @@ -839,7 +862,17 @@ int nvdimm_bus_check_dimm_count(struct nvdimm_bus *nvdimm_bus, int dimm_count)
>>  }
>>  EXPORT_SYMBOL_GPL(nvdimm_bus_check_dimm_count);
>>
>> +int __init nvdimm_devs_init(void)
>> +{
>> +       nvdimm_wq = create_singlethread_workqueue("nvdimm");
>> +       if (!nvdimm_wq)
>> +               return -ENOMEM;
>> +
>> +       return 0;
>> +}
>> +
>>  void __exit nvdimm_devs_exit(void)
>>  {
>> +       destroy_workqueue(nvdimm_wq);
> 
> I don't see a call to cancel_delayed_work_sync(), what ensures that
> the dimm is not hot removed while overwrite is in flight? It's
> probably ok to not wait for completion because the driver will
> reacquire the state at the next driver load, but the work needs to be
> cancelled, shutoff, and flushed before the device_del() of the nvdimm
> device.
> 
> 
> 
>>         ida_destroy(&dimm_ida);
>>  }
>> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
>> index 353f945cda5b..2c93e2139346 100644
>> --- a/drivers/nvdimm/nd-core.h
>> +++ b/drivers/nvdimm/nd-core.h
>> @@ -21,6 +21,7 @@
>>  extern struct list_head nvdimm_bus_list;
>>  extern struct mutex nvdimm_bus_list_mutex;
>>  extern int nvdimm_major;
>> +extern struct workqueue_struct *nvdimm_wq;
>>
>>  struct nvdimm_bus {
>>         struct nvdimm_bus_descriptor *nd_desc;
>> @@ -45,7 +46,10 @@ struct nvdimm {
>>         struct {
>>                 const struct nvdimm_security_ops *ops;
>>                 enum nvdimm_security_state state;
>> +               unsigned int overwrite_tmo;
>> +               struct kernfs_node *overwrite_state;
>>         } sec;
>> +       struct delayed_work dwork;
>>  };
>>
>>  static inline enum nvdimm_security_state nvdimm_security_state(
>> @@ -78,6 +82,9 @@ static inline void nvdimm_clear_security_busy(struct nvdimm *nvdimm)
>>         clear_bit(NDD_SECURITY_BUSY, &nvdimm->flags);
>>  }
>>
>> +int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid);
>> +void nvdimm_security_overwrite_query(struct work_struct *work);
>> +
>>  /**
>>   * struct blk_alloc_info - tracking info for BLK dpa scanning
>>   * @nd_mapping: blk region mapping boundaries
>> @@ -110,6 +117,7 @@ static inline bool is_memory(struct device *dev)
>>  struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev);
>>  int __init nvdimm_bus_init(void);
>>  void nvdimm_bus_exit(void);
>> +int nvdimm_devs_init(void);
>>  void nvdimm_devs_exit(void);
>>  void nd_region_devs_exit(void);
>>  void nd_region_probe_success(struct nvdimm_bus *nvdimm_bus, struct device *dev);
>> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
>> index 5a03dffd0056..3603c5fda1cf 100644
>> --- a/drivers/nvdimm/security.c
>> +++ b/drivers/nvdimm/security.c
>> @@ -282,7 +282,7 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
>>         struct key *key;
>>         int rc;
>>
>> -       /* The bus lock should be held at the top level of the call stack */
>> +       /* the bus lock should be held at the top level of the call stack */
> 
> Unintended change? Capitalized sentences is the local nvdimm custom.
> 
>>         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>>
>>         if (!nvdimm->sec.ops || !nvdimm->sec.ops->erase
>> @@ -318,3 +318,99 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid)
>>         nvdimm->sec.state = nvdimm_security_state(nvdimm);
>>         return rc;
>>  }
>> +
>> +int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
>> +{
>> +       struct device *dev = &nvdimm->dev;
>> +       struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
>> +       struct key *key;
>> +       int rc;
>> +
>> +       /* the bus lock should be held at the top level of the call stack */
>> +       lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>> +
>> +       if (!nvdimm->sec.ops || !nvdimm->sec.ops->overwrite
>> +                       || nvdimm->sec.state < 0)
>> +               return -EIO;
>> +
>> +       if (atomic_read(&nvdimm->busy)) {
>> +               dev_warn(dev, "Unable to overwrite while DIMM active.\n");
>> +               nvdimm_clear_security_busy(nvdimm);
>> +               return -EBUSY;
>> +       }
>> +
>> +       if (dev_get_drvdata(dev)) {
> 
> Use "dev->driver != NULL" as the "is enabled" test. dev_get_drvdata()
> works today, but I've seen driver-core patches that tried to make
> assumptions about the availability of drvdata for the core to use
> while dev->driver is NULL.
> 
> 
>> +               dev_warn(dev, "Unable to overwrite while DIMM active.\n");
>> +               nvdimm_clear_security_busy(nvdimm);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (nvdimm->sec.state >= NVDIMM_SECURITY_FROZEN) {
>> +               dev_warn(dev, "Incorrect security state: %d\n",
>> +                               nvdimm->sec.state);
>> +               return -EIO;
>> +       }
>> +
>> +       rc = nvdimm_security_check_busy(nvdimm);
>> +       if (rc < 0) {
>> +               dev_warn(dev, "Security operation in progress.\n");
>> +               return rc;
>> +       }
>> +
>> +       if (keyid == 0)
>> +               key = NULL;
>> +       else {
>> +               key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
>> +               if (!key)
>> +                       return -ENOKEY;
>> +       }
>> +
>> +       rc = nvdimm->sec.ops->overwrite(nvdimm, key ? key_data(key) : NULL);
>> +       dev_dbg(dev, "key: %d overwrite submission: %s\n", key_serial(key),
>> +                       rc == 0 ? "success" : "fail");
>> +
>> +       nvdimm_put_key(key);
>> +       nvdimm->sec.state = nvdimm_security_state(nvdimm);
>> +       nvdimm_set_security_busy(nvdimm);
>> +       return rc;
>> +}
>> +
>> +void nvdimm_security_overwrite_query(struct work_struct *work)
>> +{
>> +       struct nvdimm_bus *nvdimm_bus;
>> +       struct nvdimm *nvdimm;
>> +       int rc;
>> +       unsigned int tmo;
>> +
>> +       nvdimm = container_of(work, typeof(*nvdimm), dwork.work);
>> +       nvdimm_bus = walk_to_nvdimm_bus(&nvdimm->dev);
>> +       tmo = nvdimm->sec.overwrite_tmo;
>> +
>> +       /* The bus lock should be held at the top level of the call stack */
>> +       lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>> +
> 
> The reconfig_mutex should have been dropped after the work was queued,
> so we would need to reacquire and revalidate state here.
> 
> We'd also need to abort and not re-queue if the nvdimm device is going away.
> 
>> +       if (!nvdimm->sec.ops || !nvdimm->sec.ops->query_overwrite
>> +                       || nvdimm->sec.state < 0)
>> +               return;
>> +
>> +       rc = nvdimm->sec.ops->query_overwrite(nvdimm);
>> +       if (rc == -EBUSY) {
>> +
>> +               /* setup delayed work again */
>> +               tmo += 10;
>> +               queue_delayed_work(nvdimm_wq, &nvdimm->dwork, tmo * HZ);
>> +               nvdimm->sec.overwrite_tmo = min(15U * 60U, tmo);
>> +               return;
>> +       }
>> +
>> +       if (rc < 0)
>> +               dev_warn(&nvdimm->dev, "overwrite failed\n");
>> +       else
>> +               dev_info(&nvdimm->dev, "overwrite completed\n");
> 
> Let's do dev_dbg(), no need to be chatty on success, and the driver is
> already notifying userspace of the completion event.
> 
>> +
>> +       if (nvdimm->sec.overwrite_state)
>> +               sysfs_notify_dirent(nvdimm->sec.overwrite_state);
>> +       nvdimm->sec.overwrite_tmo = 0;
>> +       nvdimm_clear_security_busy(nvdimm);
>> +       nvdimm->sec.state = nvdimm_security_state(nvdimm);
>> +}
>> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
>> index 8507d2896ae0..b728952ccb28 100644
>> --- a/include/linux/libnvdimm.h
>> +++ b/include/linux/libnvdimm.h
>> @@ -184,6 +184,9 @@ struct nvdimm_security_ops {
>>                         const struct nvdimm_key_data *key_data);
>>         int (*erase)(struct nvdimm *nvdimm,
>>                         const struct nvdimm_key_data *key_data);
>> +       int (*overwrite)(struct nvdimm *nvdimm,
>> +                       const struct nvdimm_key_data *key_data);
>> +       int (*query_overwrite)(struct nvdimm *nvdimm);
>>  };
>>
>>  void badrange_init(struct badrange *badrange);
>> @@ -221,6 +224,7 @@ 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,
>>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys
  2018-12-11 20:25 ` [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys Dave Jiang
@ 2018-12-12 10:51   ` Mimi Zohar
  0 siblings, 0 replies; 22+ messages in thread
From: Mimi Zohar @ 2018-12-12 10:51 UTC (permalink / raw)
  To: Dave Jiang, dan.j.williams; +Cc: dhowells, zohar, linux-nvdimm

On Tue, 2018-12-11 at 13:25 -0700, Dave Jiang wrote:
> Adding nvdimm key format type to encrypted keys in order to limit the size
> of the key to 32bytes.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  Documentation/security/keys/trusted-encrypted.rst |    6 ++++
>  security/keys/encrypted-keys/encrypted.c          |   29 ++++++++++++++-------
>  2 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index 3bb24e09a332..e8a1c35cd277 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -76,7 +76,7 @@ Usage::
> 
>  Where::
> 
> -	format:= 'default | ecryptfs'
> +	format:= 'default | ecryptfs | enc32'
>  	key-type:= 'trusted' | 'user'
> 
> 
> @@ -173,3 +173,7 @@ are anticipated.  In particular the new format 'ecryptfs' has been defined in
>  in order to use encrypted keys to mount an eCryptfs filesystem.  More details
>  about the usage can be found in the file
>  ``Documentation/security/keys/ecryptfs.rst``.
> +
> +Another new format 'enc32' has been defined in order to support encrypted keys
> +with payload size of 32 bytes. This will initially be used for nvdimm security
> +but may expand to other usages that require 32 bytes payload.
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index d92cbf9687c3..fe0aefd06f83 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -45,6 +45,7 @@ static const char hmac_alg[] = "hmac(sha256)";
>  static const char blkcipher_alg[] = "cbc(aes)";
>  static const char key_format_default[] = "default";
>  static const char key_format_ecryptfs[] = "ecryptfs";
> +static const char key_format_enc32[] = "enc32";
>  static unsigned int ivsize;
>  static int blksize;
> 
> @@ -54,6 +55,7 @@ static int blksize;
>  #define HASH_SIZE SHA256_DIGEST_SIZE
>  #define MAX_DATA_SIZE 4096
>  #define MIN_DATA_SIZE  20
> +#define KEY_ENC32_PAYLOAD_LEN 32
> 
>  static struct crypto_shash *hash_tfm;
> 
> @@ -62,12 +64,13 @@ enum {
>  };
> 
>  enum {
> -	Opt_error = -1, Opt_default, Opt_ecryptfs
> +	Opt_error = -1, Opt_default, Opt_ecryptfs, Opt_enc32
>  };
> 
>  static const match_table_t key_format_tokens = {
>  	{Opt_default, "default"},
>  	{Opt_ecryptfs, "ecryptfs"},
> +	{Opt_enc32, "enc32"},
>  	{Opt_error, NULL}
>  };
> 
> @@ -195,6 +198,7 @@ static int datablob_parse(char *datablob, const char **format,
>  	key_format = match_token(p, key_format_tokens, args);
>  	switch (key_format) {
>  	case Opt_ecryptfs:
> +	case Opt_enc32:
>  	case Opt_default:
>  		*format = p;
>  		*master_desc = strsep(&datablob, " \t");
> @@ -625,15 +629,22 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
>  	format_len = (!format) ? strlen(key_format_default) : strlen(format);
>  	decrypted_datalen = dlen;
>  	payload_datalen = decrypted_datalen;
> -	if (format && !strcmp(format, key_format_ecryptfs)) {
> -		if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
> -			pr_err("encrypted_key: keylen for the ecryptfs format "
> -			       "must be equal to %d bytes\n",
> -			       ECRYPTFS_MAX_KEY_BYTES);
> -			return ERR_PTR(-EINVAL);
> +	if (format) {
> +		if (!strcmp(format, key_format_ecryptfs)) {
> +			if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
> +				pr_err("encrypted_key: keylen for the ecryptfs format must be equal to %d bytes\n",
> +					ECRYPTFS_MAX_KEY_BYTES);
> +				return ERR_PTR(-EINVAL);
> +			}
> +			decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
> +			payload_datalen = sizeof(struct ecryptfs_auth_tok);
> +		} else if (!strcmp(format, key_format_enc32)) {
> +			if (decrypted_datalen != KEY_ENC32_PAYLOAD_LEN) {
> +				pr_err("encrypted_key: enc32 key payload incorrect length: %d\n",
> +						decrypted_datalen);
> +				return ERR_PTR(-EINVAL);
> +			}
>  		}
> -		decrypted_datalen = ECRYPTFS_MAX_KEY_BYTES;
> -		payload_datalen = sizeof(struct ecryptfs_auth_tok);
>  	}
> 
>  	encrypted_datalen = roundup(decrypted_datalen, blksize);
> 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-12-12 10:51 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 20:25 [PATCH v13 00/17] Adding security support for nvdimm Dave Jiang
2018-12-11 20:25 ` [PATCH v13 01/17] acpi/nfit: Add support for Intel DSM 1.8 commands Dave Jiang
2018-12-11 20:25 ` [PATCH v13 02/17] acpi/nfit, libnvdimm: Store dimm id as a member to struct nvdimm Dave Jiang
2018-12-11 20:25 ` [PATCH v13 03/17] keys: Export lookup_user_key to external users Dave Jiang
2018-12-11 20:25 ` [PATCH v13 04/17] keys-encrypted: add nvdimm key format type to encrypted keys Dave Jiang
2018-12-12 10:51   ` Mimi Zohar
2018-12-11 20:25 ` [PATCH v13 05/17] acpi/nfit, libnvdimm: Introduce nvdimm_security_ops Dave Jiang
2018-12-11 20:25 ` [PATCH v13 06/17] acpi/nfit, libnvdimm: Add freeze security support to Intel nvdimm Dave Jiang
2018-12-11 20:26 ` [PATCH v13 07/17] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs Dave Jiang
2018-12-11 20:26 ` [PATCH v13 08/17] acpi/nfit, libnvdimm: Add disable passphrase support to Intel nvdimm Dave Jiang
2018-12-11 20:26 ` [PATCH v13 09/17] acpi/nfit, libnvdimm: Add enable/update passphrase support for Intel nvdimms Dave Jiang
2018-12-11 20:26 ` [PATCH v13 10/17] acpi/nfit, libnvdimm: Add support for issue secure erase DSM to Intel nvdimm Dave Jiang
2018-12-11 20:26 ` [PATCH v13 11/17] libnvdimm/security: introduce NDD_SECURITY_BUSY flag Dave Jiang
2018-12-11 20:26 ` [PATCH v13 12/17] acpi/nfit, libnvdimm/security: Add security DSM overwrite support Dave Jiang
2018-12-11 23:44   ` Dan Williams
2018-12-12  0:33     ` Dave Jiang
2018-12-11 20:26 ` [PATCH v13 13/17] acpi/nfit, libnvdimm/security: add Intel DSM 1.8 master passphrase support Dave Jiang
2018-12-11 23:30   ` Dan Williams
2018-12-11 20:26 ` [PATCH v13 14/17] tools/testing/nvdimm: Add test support for Intel nvdimm security DSMs Dave Jiang
2018-12-11 20:26 ` [PATCH v13 15/17] tools/testing/nvdimm: Add overwrite support for nfit_test Dave Jiang
2018-12-11 20:26 ` [PATCH v13 16/17] tools/testing/nvdimm: add Intel DSM 1.8 " Dave Jiang
2018-12-11 20:26 ` [PATCH v13 17/17] libnvdimm/security: Add documentation for nvdimm security support Dave Jiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).