@@ -150,6 +150,7 @@ struct key_type {
/* internal fields */
struct list_head link; /* link in types list */
struct lock_class_key lock_class; /* key->sem lock class */
+ struct rw_semaphore sem; /* for safe key type removal */
};
extern struct key_type key_type_keyring;
@@ -99,7 +99,7 @@ static void key_gc_timer_func(unsigned long data)
* collect dead links and the third to clean up the dead keys. We have to be
* careful as there may already be a cycle in progress.
*
- * The caller must be holding key_types_sem.
+ * The caller must be holding key_types_sem and ktype->sem.
*/
void key_gc_keytype(struct key_type *ktype)
{
@@ -672,8 +672,8 @@ struct key *key_lookup(key_serial_t id)
/*
* Find and lock the specified key type against removal.
*
- * We return with the sem read-locked if successful. If the type wasn't
- * available -ENOKEY is returned instead.
+ * We return with the key type's sem read-locked if successful. If the
+ * type wasn't available -ENOKEY is returned instead.
*/
struct key_type *key_type_lookup(const char *type)
{
@@ -684,14 +684,18 @@ struct key_type *key_type_lookup(const char *type)
/* look up the key type to see if it's one of the registered kernel
* types */
list_for_each_entry(ktype, &key_types_list, link) {
- if (strcmp(ktype->name, type) == 0)
+ /* ktype->sem is only write locked when unregistering, so
+ * ignore a key type with a contended lock.
+ */
+ if ((strcmp(ktype->name, type) == 0) &&
+ down_read_trylock(&ktype->sem))
goto found_kernel_type;
}
- up_read(&key_types_sem);
ktype = ERR_PTR(-ENOKEY);
found_kernel_type:
+ up_read(&key_types_sem);
return ktype;
}
@@ -720,7 +724,8 @@ EXPORT_SYMBOL_GPL(key_set_timeout);
*/
void key_type_put(struct key_type *ktype)
{
- up_read(&key_types_sem);
+ if (ktype)
+ up_read(&ktype->sem);
}
/*
@@ -1102,6 +1107,7 @@ int register_key_type(struct key_type *ktype)
int ret;
memset(&ktype->lock_class, 0, sizeof(ktype->lock_class));
+ init_rwsem(&ktype->sem);
ret = -EEXIST;
down_write(&key_types_sem);
@@ -1135,14 +1141,22 @@ EXPORT_SYMBOL(register_key_type);
void unregister_key_type(struct key_type *ktype)
{
down_write(&key_types_sem);
+ down_write(&ktype->sem);
list_del_init(&ktype->link);
downgrade_write(&key_types_sem);
key_gc_keytype(ktype);
pr_notice("Key type %s unregistered\n", ktype->name);
+ up_write(&ktype->sem);
up_read(&key_types_sem);
}
EXPORT_SYMBOL(unregister_key_type);
+static void __init add_special_key_type(struct key_type *ktype)
+{
+ init_rwsem(&ktype->sem);
+ list_add_tail(&ktype->link, &key_types_list);
+}
+
/*
* Initialise the key management state.
*/
@@ -1153,10 +1167,10 @@ void __init key_init(void)
0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
/* add the special key types */
- list_add_tail(&key_type_keyring.link, &key_types_list);
- list_add_tail(&key_type_dead.link, &key_types_list);
- list_add_tail(&key_type_user.link, &key_types_list);
- list_add_tail(&key_type_logon.link, &key_types_list);
+ add_special_key_type(&key_type_keyring);
+ add_special_key_type(&key_type_dead);
+ add_special_key_type(&key_type_user);
+ add_special_key_type(&key_type_logon);
/* record the root user tracking */
rb_link_node(&root_key_user.node,
The key_types_sem read-write semaphore has been performing two jobs, protecting the key_types_list and making sure individual key types are not unregistered while they are in use. It is held for most of key_create_or_update, which means a second key type cannot be looked up by name while a key is being created or updated without re-acquiring the semaphore and triggering a lockdep warning: [ 445.752215] ============================================= [ 445.752946] [ INFO: possible recursive locking detected ] [ 445.753745] 4.8.0-rc1+ #34 Not tainted [ 445.754087] --------------------------------------------- [ 445.754540] test-key/948 is trying to acquire lock: [ 445.754950] (key_types_sem){++++.+}, at: [<ffffffff903c4dcb>] key_type_lookup+0x1b/0x80 [ 445.755854] [ 445.755854] but task is already holding lock: [ 445.756515] (key_types_sem){++++.+}, at: [<ffffffff903c4dcb>] key_type_lookup+0x1b/0x80 By adding a rwsem to struct key_type, key_types_sem only needs to be held when key_types_list is being accessed. key_type_put() is modified to only release the given key_type (the key_type parameter to this function was previously ignored). Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com> --- include/linux/key-type.h | 1 + security/keys/gc.c | 2 +- security/keys/key.c | 32 +++++++++++++++++++++++--------- 3 files changed, 25 insertions(+), 10 deletions(-)