MIRRORS/linux-imx
1
0
Fork 0
mirror of https://github.com/nxp-imx/linux-imx.git synced 2025-07-07 01:45:18 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

ANDROID: fips140: fix deadlock in unregister_existing_fips140_algos()

Browse Source 
crypto_remove_final() calls crypto_alg_put() which can take
crypto_alg_sem again, via a call stack like:

    down_write(&crypto_alg_sem)
    crypto_drop_spawn()
    crypto_ccm_free()
    crypto_aead_free_instance()
    crypto_destroy_instance()
    crypto_alg_put() (inlined)
    crypto_remove_final()
    unregister_existing_fips140_algos()

That causes a deadlock because unregister_existing_fips140_algos() is
already holding crypto_alg_sem.

Fix this by reducing the scope of crypto_alg_sem to the actual list
traversal and not the crypto_alg_put().

Bug: 153614920
Bug: 188620248
Change-Id: Ia724d8b13480233dad051c538dc504cb27be8777
Signed-off-by: Eric Biggers <ebiggers@google.com>
This commit is contained in:
Eric Biggers 2021-07-08 14:46:43 -07:00 committed by Ard Biesheuvel
parent 0af06624ea
commit 634445a640
1 changed files with 35 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
crypto/fips140-module.c
View File

@ -116,6 +116,38 @@ static bool __init is_fips140_algo(struct crypto_alg *alg)
static LIST_HEAD(unchecked_fips140_algos); static LIST_HEAD(unchecked_fips140_algos);
/*
* Release a list of algorithms which have been removed from crypto_alg_list.
*
* Note that even though the list is a private list, we have to hold
* crypto_alg_sem while iterating through it because crypto_unregister_alg() may
* run concurrently (as we haven't taken a reference to the algorithms on the
* list), and crypto_unregister_alg() will remove the algorithm from whichever
* list it happens to be on, while holding crypto_alg_sem. That's okay, since
* in that case crypto_unregister_alg() will handle the crypto_alg_put().
*/
static void fips140_remove_final(struct list_head *list)
{
struct crypto_alg *alg;
struct crypto_alg *n;
/*
* We need to take crypto_alg_sem to safely traverse the list (see
* comment above), but we have to drop it when doing each
* crypto_alg_put() as that may take crypto_alg_sem again.
*/
down_write(&crypto_alg_sem);
list_for_each_entry_safe(alg, n, list, cra_list) {
list_del_init(&alg->cra_list);
up_write(&crypto_alg_sem);
crypto_alg_put(alg);
down_write(&crypto_alg_sem);
}
up_write(&crypto_alg_sem);
}
static void __init unregister_existing_fips140_algos(void) static void __init unregister_existing_fips140_algos(void)
{ {
struct crypto_alg *alg, *tmp; struct crypto_alg *alg, *tmp;
@ -159,20 +191,10 @@ static void __init unregister_existing_fips140_algos(void)
} }
} }
} }
/*
* We haven't taken a reference to the algorithms on the remove_list,
* so technically, we may be competing with a concurrent invocation of
* crypto_unregister_alg() here. Fortunately, crypto_unregister_alg()
* just gives up with a warning if the algo that is being unregistered
* has already disappeared, so this happens to be safe. That does mean
* we need to hold on to the lock, to ensure that the algo is either on
* the list or it is not, and not in some limbo state.
*/
crypto_remove_final(&remove_list);
crypto_remove_final(&spawns);
up_write(&crypto_alg_sem); up_write(&crypto_alg_sem);
fips140_remove_final(&remove_list);
fips140_remove_final(&spawns);
} }
static void __init unapply_text_relocations(void *section, int section_size, static void __init unapply_text_relocations(void *section, int section_size,