3.16.52-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Biggers <ebigg...@google.com>

commit ee618b4619b72527aaed765f0f0b74072b281159 upstream.

As the previous patch did for encrypted-keys, zero sensitive any
potentially sensitive data related to the "trusted" key type before it
is freed.  Notably, we were not zeroing the tpm_buf structures in which
the actual key is stored for TPM seal and unseal, nor were we zeroing
the trusted_key_payload in certain error paths.

Cc: Mimi Zohar <zo...@linux.vnet.ibm.com>
Cc: David Safford <saff...@us.ibm.com>
Signed-off-by: Eric Biggers <ebigg...@google.com>
Signed-off-by: David Howells <dhowe...@redhat.com>
Signed-off-by: James Morris <james.l.mor...@oracle.com>
[bwh: Backported to 3.16:
 - Drop one unapplicable change
 - Adjust context]
Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
---
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -69,7 +69,7 @@ static int TSS_sha1(const unsigned char
        }
 
        ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest);
-       kfree(sdesc);
+       kzfree(sdesc);
        return ret;
 }
 
@@ -113,7 +113,7 @@ static int TSS_rawhmac(unsigned char *di
        if (!ret)
                ret = crypto_shash_final(&sdesc->shash, digest);
 out:
-       kfree(sdesc);
+       kzfree(sdesc);
        return ret;
 }
 
@@ -164,7 +164,7 @@ static int TSS_authhmac(unsigned char *d
                                  paramdigest, TPM_NONCE_SIZE, h1,
                                  TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
 out:
-       kfree(sdesc);
+       kzfree(sdesc);
        return ret;
 }
 
@@ -245,7 +245,7 @@ static int TSS_checkhmac1(unsigned char
        if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE))
                ret = -EINVAL;
 out:
-       kfree(sdesc);
+       kzfree(sdesc);
        return ret;
 }
 
@@ -346,7 +346,7 @@ static int TSS_checkhmac2(unsigned char
        if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE))
                ret = -EINVAL;
 out:
-       kfree(sdesc);
+       kzfree(sdesc);
        return ret;
 }
 
@@ -563,7 +563,7 @@ static int tpm_seal(struct tpm_buf *tb,
                *bloblen = storedsize;
        }
 out:
-       kfree(td);
+       kzfree(td);
        return ret;
 }
 
@@ -677,7 +677,7 @@ static int key_seal(struct trusted_key_p
        if (ret < 0)
                pr_info("trusted_key: srkseal failed (%d)\n", ret);
 
-       kfree(tb);
+       kzfree(tb);
        return ret;
 }
 
@@ -702,7 +702,7 @@ static int key_unseal(struct trusted_key
                /* pull migratable flag out of sealed key */
                p->migratable = p->key[--p->key_len];
 
-       kfree(tb);
+       kzfree(tb);
        return ret;
 }
 
@@ -961,12 +961,12 @@ static int trusted_instantiate(struct ke
        if (!ret && options->pcrlock)
                ret = pcrlock(options->pcrlock);
 out:
-       kfree(datablob);
-       kfree(options);
+       kzfree(datablob);
+       kzfree(options);
        if (!ret)
                rcu_assign_keypointer(key, payload);
        else
-               kfree(payload);
+               kzfree(payload);
        return ret;
 }
 
@@ -975,8 +975,7 @@ static void trusted_rcu_free(struct rcu_
        struct trusted_key_payload *p;
 
        p = container_of(rcu, struct trusted_key_payload, rcu);
-       memset(p->key, 0, p->key_len);
-       kfree(p);
+       kzfree(p);
 }
 
 /*
@@ -1015,7 +1014,7 @@ static int trusted_update(struct key *ke
        ret = datablob_parse(datablob, new_p, new_o);
        if (ret != Opt_update) {
                ret = -EINVAL;
-               kfree(new_p);
+               kzfree(new_p);
                goto out;
        }
        /* copy old key values, and reseal with new pcrs */
@@ -1028,22 +1027,22 @@ static int trusted_update(struct key *ke
        ret = key_seal(new_p, new_o);
        if (ret < 0) {
                pr_info("trusted_key: key_seal failed (%d)\n", ret);
-               kfree(new_p);
+               kzfree(new_p);
                goto out;
        }
        if (new_o->pcrlock) {
                ret = pcrlock(new_o->pcrlock);
                if (ret < 0) {
                        pr_info("trusted_key: pcrlock failed (%d)\n", ret);
-                       kfree(new_p);
+                       kzfree(new_p);
                        goto out;
                }
        }
        rcu_assign_keypointer(key, new_p);
        call_rcu(&p->rcu, trusted_rcu_free);
 out:
-       kfree(datablob);
-       kfree(new_o);
+       kzfree(datablob);
+       kzfree(new_o);
        return ret;
 }
 
@@ -1072,24 +1071,19 @@ static long trusted_read(const struct ke
        for (i = 0; i < p->blob_len; i++)
                bufp = hex_byte_pack(bufp, p->blob[i]);
        if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) {
-               kfree(ascii_buf);
+               kzfree(ascii_buf);
                return -EFAULT;
        }
-       kfree(ascii_buf);
+       kzfree(ascii_buf);
        return 2 * p->blob_len;
 }
 
 /*
- * trusted_destroy - before freeing the key, clear the decrypted data
+ * trusted_destroy - clear and free the key's payload
  */
 static void trusted_destroy(struct key *key)
 {
-       struct trusted_key_payload *p = key->payload.data;
-
-       if (!p)
-               return;
-       memset(p->key, 0, p->key_len);
-       kfree(key->payload.data);
+       kzfree(key->payload.data);
 }
 
 struct key_type key_type_trusted = {

Reply via email to