Merge tag 'libnvdimm-fixes-5.1-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm

Pull libnvdimm fixes from Dan Williams:
 "I debated holding this back for the v5.2 merge window due to the size
  of the "zero-key" changes, but affected users would benefit from
  having the fixes sooner. It did not make sense to change the zero-key
  semantic in isolation for the "secure-erase" command, but instead
  include it for all security commands.

  The short background on the need for these changes is that some NVDIMM
  platforms enable security with a default zero-key rather than let the
  OS specify the initial key. This makes the security enabling that
  landed in v5.0 unusable for some users.

  Summary:

   - Compatibility fix for nvdimm-security implementations with a
     default zero-key.

   - Miscellaneous small fixes for out-of-bound accesses, cleanup after
     initialization failures, and missing debug messages"

* tag 'libnvdimm-fixes-5.1-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm:
  tools/testing/nvdimm: Retain security state after overwrite
  libnvdimm/pmem: fix a possible OOB access when read and write pmem
  libnvdimm/security, acpi/nfit: unify zero-key for all security commands
  libnvdimm/security: provide fix for secure-erase to use zero-key
  libnvdimm/btt: Fix a kmemdup failure check
  libnvdimm/namespace: Fix a potential NULL pointer dereference
  acpi/nfit: Always dump _DSM output payload

7 files changed, 117 insertions, 71 deletions

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5a389a4f4f65..f1ed0befe303 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -567,6 +567,12 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
                 goto out;
         }
 
+        dev_dbg(dev, "%s cmd: %s output length: %d\n", dimm_name,
+                        cmd_name, out_obj->buffer.length);
+        print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
+                        out_obj->buffer.pointer,
+                        min_t(u32, 128, out_obj->buffer.length), true);
+
         if (call_pkg) {
                 call_pkg->nd_fw_size = out_obj->buffer.length;
                 memcpy(call_pkg->nd_payload + call_pkg->nd_size_in,
@@ -585,12 +591,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
                 return 0;
         }
 
-        dev_dbg(dev, "%s cmd: %s output length: %d\n", dimm_name,
-                        cmd_name, out_obj->buffer.length);
-        print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
-                        out_obj->buffer.pointer,
-                        min_t(u32, 128, out_obj->buffer.length), true);
-
         for (i = 0, offset = 0; i < desc->out_num; i++) {
                 u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i, buf,
                                 (u32 *) out_obj->buffer.pointer,
diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index f70de71f79d6..cddd0fcf622c 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -122,9 +122,8 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
         if (!test_bit(cmd, &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.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);
@@ -336,9 +335,8 @@ static int __maybe_unused intel_security_overwrite(struct nvdimm *nvdimm,
 
         /* flush all cache before we erase DIMM */
         nvdimm_invalidate_cache();
-        if (nkey)
-                memcpy(nd_cmd.cmd.passphrase, nkey->data,
-                                sizeof(nd_cmd.cmd.passphrase));
+        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;
diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
index b72a303176c7..9486acc08402 100644
--- a/drivers/nvdimm/btt_devs.c
+++ b/drivers/nvdimm/btt_devs.c
@@ -198,14 +198,15 @@ static struct device *__nd_btt_create(struct nd_region *nd_region,
                 return NULL;
 
         nd_btt->id = ida_simple_get(&nd_region->btt_ida, 0, 0, GFP_KERNEL);
-        if (nd_btt->id < 0) {
-                kfree(nd_btt);
-                return NULL;
-        }
+        if (nd_btt->id < 0)
+                goto out_nd_btt;
 
         nd_btt->lbasize = lbasize;
-        if (uuid)
+        if (uuid) {
                 uuid = kmemdup(uuid, 16, GFP_KERNEL);
+                if (!uuid)
+                        goto out_put_id;
+        }
         nd_btt->uuid = uuid;
         dev = &nd_btt->dev;
         dev_set_name(dev, "btt%d.%d", nd_region->id, nd_btt->id);
@@ -220,6 +221,13 @@ static struct device *__nd_btt_create(struct nd_region *nd_region,
                 return NULL;
         }
         return dev;
+
+out_put_id:
+        ida_simple_remove(&nd_region->btt_ida, nd_btt->id);
+
+out_nd_btt:
+        kfree(nd_btt);
+        return NULL;
 }
 
 struct device *nd_btt_create(struct nd_region *nd_region)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 7849bf1812c4..f293556cbbf6 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -2249,9 +2249,12 @@ static struct device *create_namespace_blk(struct nd_region *nd_region,
         if (!nsblk->uuid)
                 goto blk_err;
         memcpy(name, nd_label->name, NSLABEL_NAME_LEN);
-        if (name[0])
+        if (name[0]) {
                 nsblk->alt_name = kmemdup(name, NSLABEL_NAME_LEN,
                                 GFP_KERNEL);
+                if (!nsblk->alt_name)
+                        goto blk_err;
+        }
         res = nsblk_add_resource(nd_region, ndd, nsblk,
                         __le64_to_cpu(nd_label->dpa));
         if (!res)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index bc2f700feef8..0279eb1da3ef 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -113,13 +113,13 @@ static void write_pmem(void *pmem_addr, struct page *page,
 
         while (len) {
                 mem = kmap_atomic(page);
-                chunk = min_t(unsigned int, len, PAGE_SIZE);
+                chunk = min_t(unsigned int, len, PAGE_SIZE - off);
                 memcpy_flushcache(pmem_addr, mem + off, chunk);
                 kunmap_atomic(mem);
                 len -= chunk;
                 off = 0;
                 page++;
-                pmem_addr += PAGE_SIZE;
+                pmem_addr += chunk;
         }
 }
 
@@ -132,7 +132,7 @@ static blk_status_t read_pmem(struct page *page, unsigned int off,
 
         while (len) {
                 mem = kmap_atomic(page);
-                chunk = min_t(unsigned int, len, PAGE_SIZE);
+                chunk = min_t(unsigned int, len, PAGE_SIZE - off);
                 rem = memcpy_mcsafe(mem + off, pmem_addr, chunk);
                 kunmap_atomic(mem);
                 if (rem)
@@ -140,7 +140,7 @@ static blk_status_t read_pmem(struct page *page, unsigned int off,
                 len -= chunk;
                 off = 0;
                 page++;
-                pmem_addr += PAGE_SIZE;
+                pmem_addr += chunk;
         }
         return BLK_STS_OK;
 }
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index f8bb746a549f..a570f2263a42 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -22,6 +22,8 @@ static bool key_revalidate = true;
 module_param(key_revalidate, bool, 0444);
 MODULE_PARM_DESC(key_revalidate, "Require key validation at init.");
 
+static const char zero_key[NVDIMM_PASSPHRASE_LEN];
+
 static void *key_data(struct key *key)
 {
         struct encrypted_key_payload *epayload = dereference_key_locked(key);
@@ -75,6 +77,16 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
         return key;
 }
 
+static const void *nvdimm_get_key_payload(struct nvdimm *nvdimm,
+                struct key **key)
+{
+        *key = nvdimm_request_key(nvdimm);
+        if (!*key)
+                return zero_key;
+
+        return key_data(*key);
+}
+
 static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
                 key_serial_t id, int subclass)
 {
@@ -105,36 +117,57 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
         return key;
 }
 
-static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
+static const void *nvdimm_get_user_key_payload(struct nvdimm *nvdimm,
+                key_serial_t id, int subclass, struct key **key)
+{
+        *key = NULL;
+        if (id == 0) {
+                if (subclass == NVDIMM_BASE_KEY)
+                        return zero_key;
+                else
+                        return NULL;
+        }
+
+        *key = nvdimm_lookup_user_key(nvdimm, id, subclass);
+        if (!*key)
+                return NULL;
+
+        return key_data(*key);
+}
+
+
+static int nvdimm_key_revalidate(struct nvdimm *nvdimm)
 {
         struct key *key;
         int rc;
+        const void *data;
 
         if (!nvdimm->sec.ops->change_key)
-                return NULL;
+                return -EOPNOTSUPP;
 
-        key = nvdimm_request_key(nvdimm);
-        if (!key)
-                return NULL;
+        data = nvdimm_get_key_payload(nvdimm, &key);
 
         /*
          * 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), NVDIMM_USER);
+        rc = nvdimm->sec.ops->change_key(nvdimm, data, data, NVDIMM_USER);
         if (rc < 0) {
                 nvdimm_put_key(key);
-                key = NULL;
+                return rc;
         }
-        return key;
+
+        nvdimm_put_key(key);
+        nvdimm->sec.state = nvdimm_security_state(nvdimm, NVDIMM_USER);
+        return 0;
 }
 
 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;
+        struct key *key;
+        const void *data;
         int rc;
 
         /* The bus lock should be held at the top level of the call stack */
@@ -160,16 +193,11 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
                 if (!key_revalidate)
                         return 0;
 
-                key = nvdimm_key_revalidate(nvdimm);
-                if (!key)
-                        return nvdimm_security_freeze(nvdimm);
+                return nvdimm_key_revalidate(nvdimm);
         } else
-                key = nvdimm_request_key(nvdimm);
+                data = nvdimm_get_key_payload(nvdimm, &key);
 
-        if (!key)
-                return -ENOKEY;
-
-        rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
+        rc = nvdimm->sec.ops->unlock(nvdimm, data);
         dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
                         rc == 0 ? "success" : "fail");
 
@@ -195,6 +223,7 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
         struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
         struct key *key;
         int rc;
+        const void *data;
 
         /* The bus lock should be held at the top level of the call stack */
         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
@@ -214,11 +243,12 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid)
                 return -EBUSY;
         }
 
-        key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
-        if (!key)
+        data = nvdimm_get_user_key_payload(nvdimm, keyid,
+                        NVDIMM_BASE_KEY, &key);
+        if (!data)
                 return -ENOKEY;
 
-        rc = nvdimm->sec.ops->disable(nvdimm, key_data(key));
+        rc = nvdimm->sec.ops->disable(nvdimm, data);
         dev_dbg(dev, "key: %d disable: %s\n", key_serial(key),
                         rc == 0 ? "success" : "fail");
 
@@ -235,6 +265,7 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
         struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
         struct key *key, *newkey;
         int rc;
+        const void *data, *newdata;
 
         /* The bus lock should be held at the top level of the call stack */
         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
@@ -249,22 +280,19 @@ int nvdimm_security_update(struct nvdimm *nvdimm, unsigned int keyid,
                 return -EIO;
         }
 
-        if (keyid == 0)
-                key = NULL;
-        else {
-                key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
-                if (!key)
-                        return -ENOKEY;
-        }
+        data = nvdimm_get_user_key_payload(nvdimm, keyid,
+                        NVDIMM_BASE_KEY, &key);
+        if (!data)
+                return -ENOKEY;
 
-        newkey = nvdimm_lookup_user_key(nvdimm, new_keyid, NVDIMM_NEW_KEY);
-        if (!newkey) {
+        newdata = nvdimm_get_user_key_payload(nvdimm, new_keyid,
+                        NVDIMM_NEW_KEY, &newkey);
+        if (!newdata) {
                 nvdimm_put_key(key);
                 return -ENOKEY;
         }
 
-        rc = nvdimm->sec.ops->change_key(nvdimm, key ? key_data(key) : NULL,
-                        key_data(newkey), pass_type);
+        rc = nvdimm->sec.ops->change_key(nvdimm, data, newdata, pass_type);
         dev_dbg(dev, "key: %d %d update%s: %s\n",
                         key_serial(key), key_serial(newkey),
                         pass_type == NVDIMM_MASTER ? "(master)" : "(user)",
@@ -286,8 +314,9 @@ 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;
+        struct key *key = NULL;
         int rc;
+        const void *data;
 
         /* The bus lock should be held at the top level of the call stack */
         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
@@ -319,11 +348,12 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid,
                 return -EOPNOTSUPP;
         }
 
-        key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
-        if (!key)
+        data = nvdimm_get_user_key_payload(nvdimm, keyid,
+                        NVDIMM_BASE_KEY, &key);
+        if (!data)
                 return -ENOKEY;
 
-        rc = nvdimm->sec.ops->erase(nvdimm, key_data(key), pass_type);
+        rc = nvdimm->sec.ops->erase(nvdimm, data, pass_type);
         dev_dbg(dev, "key: %d erase%s: %s\n", key_serial(key),
                         pass_type == NVDIMM_MASTER ? "(master)" : "(user)",
                         rc == 0 ? "success" : "fail");
@@ -337,8 +367,9 @@ 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;
+        struct key *key = NULL;
         int rc;
+        const void *data;
 
         /* The bus lock should be held at the top level of the call stack */
         lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
@@ -368,15 +399,12 @@ int nvdimm_security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
                 return -EBUSY;
         }
 
-        if (keyid == 0)
-                key = NULL;
-        else {
-                key = nvdimm_lookup_user_key(nvdimm, keyid, NVDIMM_BASE_KEY);
-                if (!key)
-                        return -ENOKEY;
-        }
+        data = nvdimm_get_user_key_payload(nvdimm, keyid,
+                        NVDIMM_BASE_KEY, &key);
+        if (!data)
+                return -ENOKEY;
 
-        rc = nvdimm->sec.ops->overwrite(nvdimm, key ? key_data(key) : NULL);
+        rc = nvdimm->sec.ops->overwrite(nvdimm, data);
         dev_dbg(dev, "key: %d overwrite submission: %s\n", key_serial(key),
                         rc == 0 ? "success" : "fail");
 
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index b579f962451d..85ffdcfa596b 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -146,6 +146,7 @@ static int dimm_fail_cmd_code[ARRAY_SIZE(handle)];
 struct nfit_test_sec {
         u8 state;
         u8 ext_state;
+        u8 old_state;
         u8 passphrase[32];
         u8 master_passphrase[32];
         u64 overwrite_end_time;
@@ -225,6 +226,8 @@ static struct workqueue_struct *nfit_wq;
 
 static struct gen_pool *nfit_pool;
 
+static const char zero_key[NVDIMM_PASSPHRASE_LEN];
+
 static struct nfit_test *to_nfit_test(struct device *dev)
 {
         struct platform_device *pdev = to_platform_device(dev);
@@ -1059,8 +1062,7 @@ static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
         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)) {
+        if (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,
@@ -1068,6 +1070,12 @@ static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
                 nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
                 dev_dbg(dev, "secure erase: wrong passphrase\n");
         } else {
+                if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED)
+                                && (memcmp(nd_cmd->passphrase, zero_key,
+                                        ND_INTEL_PASSPHRASE_SIZE) != 0)) {
+                        dev_dbg(dev, "invalid zero key\n");
+                        return 0;
+                }
                 memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
                 memset(sec->master_passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
                 sec->state = 0;
@@ -1093,7 +1101,7 @@ static int nd_intel_test_cmd_overwrite(struct nfit_test *t,
                 return 0;
         }
 
-        memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+        sec->old_state = sec->state;
         sec->state = ND_INTEL_SEC_STATE_OVERWRITE;
         dev_dbg(dev, "overwrite progressing.\n");
         sec->overwrite_end_time = get_jiffies_64() + 5 * HZ;
@@ -1115,7 +1123,8 @@ 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->state = sec->old_state;
+                sec->old_state = 0;
                 sec->ext_state = ND_INTEL_SEC_ESTATE_ENABLED;
                 dev_dbg(dev, "overwrite is complete\n");
         } else