keys: Hoist locking out of __key_link_begin()

Hoist the locking of out of __key_link_begin() and into its callers.  This
is necessary to allow the upcoming key_move() operation to correctly order
taking of the source keyring semaphore, the destination keyring semaphore
and the keyring serialisation lock.

Signed-off-by: David Howells <dhowells@redhat.com>
This commit is contained in:
David Howells 2019-05-30 11:37:39 +01:00
parent eb0f68cb70
commit df593ee23e
4 changed files with 77 additions and 39 deletions

View file

@ -93,6 +93,8 @@ extern wait_queue_head_t request_key_conswq;
extern struct key_type *key_type_lookup(const char *type);
extern void key_type_put(struct key_type *ktype);
extern int __key_link_lock(struct key *keyring,
const struct keyring_index_key *index_key);
extern int __key_link_begin(struct key *keyring,
const struct keyring_index_key *index_key,
struct assoc_array_edit **_edit);

View file

@ -500,7 +500,7 @@ int key_instantiate_and_link(struct key *key,
struct key *authkey)
{
struct key_preparsed_payload prep;
struct assoc_array_edit *edit;
struct assoc_array_edit *edit = NULL;
int ret;
memset(&prep, 0, sizeof(prep));
@ -515,10 +515,14 @@ int key_instantiate_and_link(struct key *key,
}
if (keyring) {
ret = __key_link_begin(keyring, &key->index_key, &edit);
ret = __key_link_lock(keyring, &key->index_key);
if (ret < 0)
goto error;
ret = __key_link_begin(keyring, &key->index_key, &edit);
if (ret < 0)
goto error_link_end;
if (keyring->restrict_link && keyring->restrict_link->check) {
struct key_restriction *keyres = keyring->restrict_link;
@ -570,7 +574,7 @@ int key_reject_and_link(struct key *key,
struct key *keyring,
struct key *authkey)
{
struct assoc_array_edit *edit;
struct assoc_array_edit *edit = NULL;
int ret, awaken, link_ret = 0;
key_check(key);
@ -583,7 +587,12 @@ int key_reject_and_link(struct key *key,
if (keyring->restrict_link)
return -EPERM;
link_ret = __key_link_begin(keyring, &key->index_key, &edit);
link_ret = __key_link_lock(keyring, &key->index_key);
if (link_ret == 0) {
link_ret = __key_link_begin(keyring, &key->index_key, &edit);
if (link_ret < 0)
__key_link_end(keyring, &key->index_key, edit);
}
}
mutex_lock(&key_construction_mutex);
@ -810,7 +819,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
.description = description,
};
struct key_preparsed_payload prep;
struct assoc_array_edit *edit;
struct assoc_array_edit *edit = NULL;
const struct cred *cred = current_cred();
struct key *keyring, *key = NULL;
key_ref_t key_ref;
@ -860,12 +869,18 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
}
index_key.desc_len = strlen(index_key.description);
ret = __key_link_begin(keyring, &index_key, &edit);
ret = __key_link_lock(keyring, &index_key);
if (ret < 0) {
key_ref = ERR_PTR(ret);
goto error_free_prep;
}
ret = __key_link_begin(keyring, &index_key, &edit);
if (ret < 0) {
key_ref = ERR_PTR(ret);
goto error_link_end;
}
if (restrict_link && restrict_link->check) {
ret = restrict_link->check(keyring, index_key.type,
&prep.payload, restrict_link->key);

View file

@ -1199,14 +1199,34 @@ static int keyring_detect_cycle(struct key *A, struct key *B)
return PTR_ERR(ctx.result) == -EAGAIN ? 0 : PTR_ERR(ctx.result);
}
/*
* Lock keyring for link.
*/
int __key_link_lock(struct key *keyring,
const struct keyring_index_key *index_key)
__acquires(&keyring->sem)
__acquires(&keyring_serialise_link_lock)
{
if (keyring->type != &key_type_keyring)
return -ENOTDIR;
down_write(&keyring->sem);
/* Serialise link/link calls to prevent parallel calls causing a cycle
* when linking two keyring in opposite orders.
*/
if (index_key->type == &key_type_keyring)
mutex_lock(&keyring_serialise_link_lock);
return 0;
}
/*
* Preallocate memory so that a key can be linked into to a keyring.
*/
int __key_link_begin(struct key *keyring,
const struct keyring_index_key *index_key,
struct assoc_array_edit **_edit)
__acquires(&keyring->sem)
__acquires(&keyring_serialise_link_lock)
{
struct assoc_array_edit *edit;
int ret;
@ -1215,20 +1235,13 @@ int __key_link_begin(struct key *keyring,
keyring->serial, index_key->type->name, index_key->description);
BUG_ON(index_key->desc_len == 0);
BUG_ON(*_edit != NULL);
if (keyring->type != &key_type_keyring)
return -ENOTDIR;
down_write(&keyring->sem);
*_edit = NULL;
ret = -EKEYREVOKED;
if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
goto error_krsem;
/* serialise link/link calls to prevent parallel calls causing a cycle
* when linking two keyring in opposite orders */
if (index_key->type == &key_type_keyring)
mutex_lock(&keyring_serialise_link_lock);
goto error;
/* Create an edit script that will insert/replace the key in the
* keyring tree.
@ -1239,7 +1252,7 @@ int __key_link_begin(struct key *keyring,
NULL);
if (IS_ERR(edit)) {
ret = PTR_ERR(edit);
goto error_sem;
goto error;
}
/* If we're not replacing a link in-place then we're going to need some
@ -1258,11 +1271,7 @@ int __key_link_begin(struct key *keyring,
error_cancel:
assoc_array_cancel_edit(edit);
error_sem:
if (index_key->type == &key_type_keyring)
mutex_unlock(&keyring_serialise_link_lock);
error_krsem:
up_write(&keyring->sem);
error:
kleave(" = %d", ret);
return ret;
}
@ -1312,9 +1321,6 @@ void __key_link_end(struct key *keyring,
BUG_ON(index_key->type == NULL);
kenter("%d,%s,", keyring->serial, index_key->type->name);
if (index_key->type == &key_type_keyring)
mutex_unlock(&keyring_serialise_link_lock);
if (edit) {
if (!edit->dead_leaf) {
key_payload_reserve(keyring,
@ -1323,6 +1329,9 @@ void __key_link_end(struct key *keyring,
assoc_array_cancel_edit(edit);
}
up_write(&keyring->sem);
if (index_key->type == &key_type_keyring)
mutex_unlock(&keyring_serialise_link_lock);
}
/*
@ -1358,7 +1367,7 @@ static int __key_link_check_restriction(struct key *keyring, struct key *key)
*/
int key_link(struct key *keyring, struct key *key)
{
struct assoc_array_edit *edit;
struct assoc_array_edit *edit = NULL;
int ret;
kenter("{%d,%d}", keyring->serial, refcount_read(&keyring->usage));
@ -1366,17 +1375,24 @@ int key_link(struct key *keyring, struct key *key)
key_check(keyring);
key_check(key);
ret = __key_link_begin(keyring, &key->index_key, &edit);
if (ret == 0) {
kdebug("begun {%d,%d}", keyring->serial, refcount_read(&keyring->usage));
ret = __key_link_check_restriction(keyring, key);
if (ret == 0)
ret = __key_link_check_live_key(keyring, key);
if (ret == 0)
__key_link(key, &edit);
__key_link_end(keyring, &key->index_key, edit);
}
ret = __key_link_lock(keyring, &key->index_key);
if (ret < 0)
goto error;
ret = __key_link_begin(keyring, &key->index_key, &edit);
if (ret < 0)
goto error_end;
kdebug("begun {%d,%d}", keyring->serial, refcount_read(&keyring->usage));
ret = __key_link_check_restriction(keyring, key);
if (ret == 0)
ret = __key_link_check_live_key(keyring, key);
if (ret == 0)
__key_link(key, &edit);
error_end:
__key_link_end(keyring, &key->index_key, edit);
error:
kleave(" = %d {%d,%d}", ret, keyring->serial, refcount_read(&keyring->usage));
return ret;
}

View file

@ -343,7 +343,7 @@ static int construct_alloc_key(struct keyring_search_context *ctx,
struct key_user *user,
struct key **_key)
{
struct assoc_array_edit *edit;
struct assoc_array_edit *edit = NULL;
struct key *key;
key_perm_t perm;
key_ref_t key_ref;
@ -372,6 +372,9 @@ static int construct_alloc_key(struct keyring_search_context *ctx,
set_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags);
if (dest_keyring) {
ret = __key_link_lock(dest_keyring, &ctx->index_key);
if (ret < 0)
goto link_lock_failed;
ret = __key_link_begin(dest_keyring, &ctx->index_key, &edit);
if (ret < 0)
goto link_prealloc_failed;
@ -423,6 +426,8 @@ static int construct_alloc_key(struct keyring_search_context *ctx,
return ret;
link_prealloc_failed:
__key_link_end(dest_keyring, &ctx->index_key, edit);
link_lock_failed:
mutex_unlock(&user->cons_lock);
key_put(key);
kleave(" = %d [prelink]", ret);