Commit 32627645 authored by Linus Torvalds's avatar Linus Torvalds

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

Pull key subsystem fixes from James Morris:
 "Here are a bunch of fixes for Linux keyrings, including:

   - Fix up the refcount handling now that key structs use the
     refcount_t type and the refcount_t ops don't allow a 0->1
     transition.

   - Fix a potential NULL deref after error in x509_cert_parse().

   - Don't put data for the crypto algorithms to use on the stack.

   - Fix the handling of a null payload being passed to add_key().

   - Fix incorrect cleanup an uninitialised key_preparsed_payload in
     key_update().

   - Explicit sanitisation of potentially secure data before freeing.

   - Fixes for the Diffie-Helman code"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (23 commits)
  KEYS: fix refcount_inc() on zero
  KEYS: Convert KEYCTL_DH_COMPUTE to use the crypto KPP API
  crypto : asymmetric_keys : verify_pefile:zero memory content before freeing
  KEYS: DH: add __user annotations to keyctl_kdf_params
  KEYS: DH: ensure the KDF counter is properly aligned
  KEYS: DH: don't feed uninitialized "otherinfo" into KDF
  KEYS: DH: forbid using digest_null as the KDF hash
  KEYS: sanitize key structs before freeing
  KEYS: trusted: sanitize all key material
  KEYS: encrypted: sanitize all key material
  KEYS: user_defined: sanitize key payloads
  KEYS: sanitize add_key() and keyctl() key payloads
  KEYS: fix freeing uninitialized memory in key_update()
  KEYS: fix dereferencing NULL payload with nonzero length
  KEYS: encrypted: use constant-time HMAC comparison
  KEYS: encrypted: fix race causing incorrect HMAC calculations
  KEYS: encrypted: fix buffer overread in valid_master_desc()
  KEYS: encrypted: avoid encrypting/decrypting stack buffers
  KEYS: put keyring if install_session_keyring_to_cred() fails
  KEYS: Delete an error message for a failed memory allocation in get_derived_key()
  ...
parents 6d53cefb 92347cfd
......@@ -1084,10 +1084,6 @@ config SYSVIPC_COMPAT
def_bool y
depends on COMPAT && SYSVIPC
config KEYS_COMPAT
def_bool y
depends on COMPAT && KEYS
endmenu
menu "Power management options"
......
......@@ -1199,11 +1199,6 @@ source "arch/powerpc/Kconfig.debug"
source "security/Kconfig"
config KEYS_COMPAT
bool
depends on COMPAT && KEYS
default y
source "crypto/Kconfig"
config PPC_LIB_RHEAP
......
......@@ -363,9 +363,6 @@ config COMPAT
config SYSVIPC_COMPAT
def_bool y if COMPAT && SYSVIPC
config KEYS_COMPAT
def_bool y if COMPAT && KEYS
config SMP
def_bool y
prompt "Symmetric multi-processing support"
......
......@@ -577,9 +577,6 @@ config SYSVIPC_COMPAT
depends on COMPAT && SYSVIPC
default y
config KEYS_COMPAT
def_bool y if COMPAT && KEYS
endmenu
source "net/Kconfig"
......
......@@ -2776,10 +2776,6 @@ config COMPAT_FOR_U64_ALIGNMENT
config SYSVIPC_COMPAT
def_bool y
depends on SYSVIPC
config KEYS_COMPAT
def_bool y
depends on KEYS
endif
endmenu
......
......@@ -381,7 +381,7 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
}
error:
kfree(desc);
kzfree(desc);
error_no_desc:
crypto_free_shash(tfm);
kleave(" = %d", ret);
......@@ -450,6 +450,6 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
ret = pefile_digest_pe(pebuf, pelen, &ctx);
error:
kfree(ctx.digest);
kzfree(ctx.digest);
return ret;
}
......@@ -102,6 +102,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
}
}
ret = -ENOMEM;
cert->pub->key = kmemdup(ctx->key, ctx->key_size, GFP_KERNEL);
if (!cert->pub->key)
goto error_decode;
......
......@@ -173,7 +173,6 @@ struct key {
#ifdef KEY_DEBUGGING
unsigned magic;
#define KEY_DEBUG_MAGIC 0x18273645u
#define KEY_DEBUG_MAGIC_X 0xf8e9dacbu
#endif
unsigned long flags; /* status flags (change with bitops) */
......
......@@ -70,8 +70,8 @@ struct keyctl_dh_params {
};
struct keyctl_kdf_params {
char *hashname;
char *otherinfo;
char __user *hashname;
char __user *otherinfo;
__u32 otherinfolen;
__u32 __spare[8];
};
......
......@@ -20,6 +20,10 @@ config KEYS
If you are unsure as to whether this is required, answer N.
config KEYS_COMPAT
def_bool y
depends on COMPAT && KEYS
config PERSISTENT_KEYRINGS
bool "Enable register of persistent per-UID keyrings"
depends on KEYS
......@@ -89,9 +93,9 @@ config ENCRYPTED_KEYS
config KEY_DH_OPERATIONS
bool "Diffie-Hellman operations on retained keys"
depends on KEYS
select MPILIB
select CRYPTO
select CRYPTO_HASH
select CRYPTO_DH
help
This option provides support for calculating Diffie-Hellman
public keys and shared secrets using values stored as keys
......
This diff is collapsed.
This diff is collapsed.
......@@ -158,9 +158,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
kfree(key->description);
#ifdef KEY_DEBUGGING
key->magic = KEY_DEBUG_MAGIC_X;
#endif
memzero_explicit(key, sizeof(*key));
kmem_cache_free(key_jar, key);
}
}
......
......@@ -660,14 +660,11 @@ struct key *key_lookup(key_serial_t id)
goto error;
found:
/* pretend it doesn't exist if it is awaiting deletion */
if (refcount_read(&key->usage) == 0)
goto not_found;
/* this races with key_put(), but that doesn't matter since key_put()
* doesn't actually change the key
/* A key is allowed to be looked up only if someone still owns a
* reference to it - otherwise it's awaiting the gc.
*/
__key_get(key);
if (!refcount_inc_not_zero(&key->usage))
goto not_found;
error:
spin_unlock(&key_serial_lock);
......@@ -966,12 +963,11 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
/* the key must be writable */
ret = key_permission(key_ref, KEY_NEED_WRITE);
if (ret < 0)
goto error;
return ret;
/* attempt to update it if supported */
ret = -EOPNOTSUPP;
if (!key->type->update)
goto error;
return -EOPNOTSUPP;
memset(&prep, 0, sizeof(prep));
prep.data = payload;
......
......@@ -99,7 +99,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
/* pull the payload in if one was supplied */
payload = NULL;
if (_payload) {
if (plen) {
ret = -ENOMEM;
payload = kvmalloc(plen, GFP_KERNEL);
if (!payload)
......@@ -132,7 +132,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
key_ref_put(keyring_ref);
error3:
if (payload) {
memzero_explicit(payload, plen);
kvfree(payload);
}
error2:
kfree(description);
error:
......@@ -324,7 +327,7 @@ long keyctl_update_key(key_serial_t id,
/* pull the payload in if one was supplied */
payload = NULL;
if (_payload) {
if (plen) {
ret = -ENOMEM;
payload = kmalloc(plen, GFP_KERNEL);
if (!payload)
......@@ -347,7 +350,7 @@ long keyctl_update_key(key_serial_t id,
key_ref_put(key_ref);
error2:
kfree(payload);
kzfree(payload);
error:
return ret;
}
......@@ -1093,7 +1096,10 @@ long keyctl_instantiate_key_common(key_serial_t id,
keyctl_change_reqkey_auth(NULL);
error2:
if (payload) {
memzero_explicit(payload, plen);
kvfree(payload);
}
error:
return ret;
}
......
......@@ -706,7 +706,7 @@ static bool search_nested_keyrings(struct key *keyring,
* Non-keyrings avoid the leftmost branch of the root entirely (root
* slots 1-15).
*/
ptr = ACCESS_ONCE(keyring->keys.root);
ptr = READ_ONCE(keyring->keys.root);
if (!ptr)
goto not_this_keyring;
......@@ -720,7 +720,7 @@ static bool search_nested_keyrings(struct key *keyring,
if ((shortcut->index_key[0] & ASSOC_ARRAY_FAN_MASK) != 0)
goto not_this_keyring;
ptr = ACCESS_ONCE(shortcut->next_node);
ptr = READ_ONCE(shortcut->next_node);
node = assoc_array_ptr_to_node(ptr);
goto begin_node;
}
......@@ -740,7 +740,7 @@ static bool search_nested_keyrings(struct key *keyring,
if (assoc_array_ptr_is_shortcut(ptr)) {
shortcut = assoc_array_ptr_to_shortcut(ptr);
smp_read_barrier_depends();
ptr = ACCESS_ONCE(shortcut->next_node);
ptr = READ_ONCE(shortcut->next_node);
BUG_ON(!assoc_array_ptr_is_node(ptr));
}
node = assoc_array_ptr_to_node(ptr);
......@@ -752,7 +752,7 @@ static bool search_nested_keyrings(struct key *keyring,
ascend_to_node:
/* Go through the slots in a node */
for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) {
ptr = ACCESS_ONCE(node->slots[slot]);
ptr = READ_ONCE(node->slots[slot]);
if (assoc_array_ptr_is_meta(ptr) && node->back_pointer)
goto descend_to_node;
......@@ -790,13 +790,13 @@ static bool search_nested_keyrings(struct key *keyring,
/* We've dealt with all the slots in the current node, so now we need
* to ascend to the parent and continue processing there.
*/
ptr = ACCESS_ONCE(node->back_pointer);
ptr = READ_ONCE(node->back_pointer);
slot = node->parent_slot;
if (ptr && assoc_array_ptr_is_shortcut(ptr)) {
shortcut = assoc_array_ptr_to_shortcut(ptr);
smp_read_barrier_depends();
ptr = ACCESS_ONCE(shortcut->back_pointer);
ptr = READ_ONCE(shortcut->back_pointer);
slot = shortcut->parent_slot;
}
if (!ptr)
......
......@@ -809,15 +809,14 @@ long join_session_keyring(const char *name)
ret = PTR_ERR(keyring);
goto error2;
} else if (keyring == new->session_keyring) {
key_put(keyring);
ret = 0;
goto error2;
goto error3;
}
/* we've got a keyring - now to install it */
ret = install_session_keyring_to_cred(new, keyring);
if (ret < 0)
goto error2;
goto error3;
commit_creds(new);
mutex_unlock(&key_session_mutex);
......@@ -827,6 +826,8 @@ long join_session_keyring(const char *name)
okay:
return ret;
error3:
key_put(keyring);
error2:
mutex_unlock(&key_session_mutex);
error:
......
......@@ -70,7 +70,7 @@ static int TSS_sha1(const unsigned char *data, unsigned int datalen,
}
ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest);
kfree(sdesc);
kzfree(sdesc);
return ret;
}
......@@ -114,7 +114,7 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
if (!ret)
ret = crypto_shash_final(&sdesc->shash, digest);
out:
kfree(sdesc);
kzfree(sdesc);
return ret;
}
......@@ -165,7 +165,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
paramdigest, TPM_NONCE_SIZE, h1,
TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
out:
kfree(sdesc);
kzfree(sdesc);
return ret;
}
......@@ -246,7 +246,7 @@ static int TSS_checkhmac1(unsigned char *buffer,
if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE))
ret = -EINVAL;
out:
kfree(sdesc);
kzfree(sdesc);
return ret;
}
......@@ -347,7 +347,7 @@ static int TSS_checkhmac2(unsigned char *buffer,
if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE))
ret = -EINVAL;
out:
kfree(sdesc);
kzfree(sdesc);
return ret;
}
......@@ -564,7 +564,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
*bloblen = storedsize;
}
out:
kfree(td);
kzfree(td);
return ret;
}
......@@ -678,7 +678,7 @@ static int key_seal(struct trusted_key_payload *p,
if (ret < 0)
pr_info("trusted_key: srkseal failed (%d)\n", ret);
kfree(tb);
kzfree(tb);
return ret;
}
......@@ -703,7 +703,7 @@ static int key_unseal(struct trusted_key_payload *p,
/* pull migratable flag out of sealed key */
p->migratable = p->key[--p->key_len];
kfree(tb);
kzfree(tb);
return ret;
}
......@@ -1037,12 +1037,12 @@ static int trusted_instantiate(struct key *key,
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;
}
......@@ -1051,8 +1051,7 @@ static void trusted_rcu_free(struct rcu_head *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);
}
/*
......@@ -1094,13 +1093,13 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
ret = datablob_parse(datablob, new_p, new_o);
if (ret != Opt_update) {
ret = -EINVAL;
kfree(new_p);
kzfree(new_p);
goto out;
}
if (!new_o->keyhandle) {
ret = -EINVAL;
kfree(new_p);
kzfree(new_p);
goto out;
}
......@@ -1114,22 +1113,22 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
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;
}
......@@ -1158,24 +1157,19 @@ static long trusted_read(const struct key *key, char __user *buffer,
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[0];
if (!p)
return;
memset(p->key, 0, p->key_len);
kfree(key->payload.data[0]);
kzfree(key->payload.data[0]);
}
struct key_type key_type_trusted = {
......
......@@ -86,10 +86,18 @@ EXPORT_SYMBOL_GPL(user_preparse);
*/
void user_free_preparse(struct key_preparsed_payload *prep)
{
kfree(prep->payload.data[0]);
kzfree(prep->payload.data[0]);
}
EXPORT_SYMBOL_GPL(user_free_preparse);
static void user_free_payload_rcu(struct rcu_head *head)
{
struct user_key_payload *payload;
payload = container_of(head, struct user_key_payload, rcu);
kzfree(payload);
}
/*
* update a user defined key
* - the key's semaphore is write-locked
......@@ -112,7 +120,7 @@ int user_update(struct key *key, struct key_preparsed_payload *prep)
prep->payload.data[0] = NULL;
if (zap)
kfree_rcu(zap, rcu);
call_rcu(&zap->rcu, user_free_payload_rcu);
return ret;
}
EXPORT_SYMBOL_GPL(user_update);
......@@ -130,7 +138,7 @@ void user_revoke(struct key *key)
if (upayload) {
rcu_assign_keypointer(key, NULL);
kfree_rcu(upayload, rcu);
call_rcu(&upayload->rcu, user_free_payload_rcu);
}
}
......@@ -143,7 +151,7 @@ void user_destroy(struct key *key)
{
struct user_key_payload *upayload = key->payload.data[0];
kfree(upayload);
kzfree(upayload);
}
EXPORT_SYMBOL_GPL(user_destroy);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment