From: Eric Biggers <[email protected]>
Michal Suchanek reported [1] that running the pcrypt_aead01 test from
LTP [2] in a loop and holding Ctrl-C causes a NULL dereference of
alg->cra_users.next in crypto_remove_spawns(), via crypto_del_alg().
The test repeatedly uses CRYPTO_MSG_NEWALG and CRYPTO_MSG_DELALG.
The crash occurs when the instance that CRYPTO_MSG_DELALG is trying to
unregister isn't a real registered algorithm, but rather is a "test
larval", which is a special "algorithm" added to the algorithms list
while the real algorithm is still being tested. Larvals don't have
initialized cra_users, so that causes the crash. Normally pcrypt_aead01
doesn't trigger this because CRYPTO_MSG_NEWALG waits for the algorithm
to be tested; however, CRYPTO_MSG_NEWALG returns early when interrupted.
Everything else in the "crypto user configuration" API has this same bug
too, i.e. it inappropriately allows operating on larval algorithms
(though it doesn't look like the other cases can cause a crash).
Fix this by making crypto_alg_match() exclude larval algorithms.
[1] https://lkml.kernel.org/r/[email protected]
[2] https://github.com/linux-test-project/ltp/blob/20190517/testcases/kernel/crypto/pcrypt_aead01.c
Reported-by: Michal Suchanek <[email protected]>
Fixes: a38f7907b926 ("crypto: Add userspace configuration API")
Cc: <[email protected]> # v3.2+
Cc: Steffen Klassert <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
---
crypto/crypto_user_base.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/crypto/crypto_user_base.c b/crypto/crypto_user_base.c
index e48da3b75c71d4..a89fcc530092a8 100644
--- a/crypto/crypto_user_base.c
+++ b/crypto/crypto_user_base.c
@@ -56,6 +56,9 @@ struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact)
list_for_each_entry(q, &crypto_alg_list, cra_list) {
int match = 0;
+ if (crypto_is_larval(q))
+ continue;
+
if ((q->cra_flags ^ p->cru_type) & p->cru_mask)
continue;
--
2.22.0.410.gd8fdbe21b5-goog
On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Michal Suchanek reported [1] that running the pcrypt_aead01 test from
> LTP [2] in a loop and holding Ctrl-C causes a NULL dereference of
> alg->cra_users.next in crypto_remove_spawns(), via crypto_del_alg().
> The test repeatedly uses CRYPTO_MSG_NEWALG and CRYPTO_MSG_DELALG.
>
> The crash occurs when the instance that CRYPTO_MSG_DELALG is trying to
> unregister isn't a real registered algorithm, but rather is a "test
> larval", which is a special "algorithm" added to the algorithms list
> while the real algorithm is still being tested. Larvals don't have
> initialized cra_users, so that causes the crash. Normally pcrypt_aead01
> doesn't trigger this because CRYPTO_MSG_NEWALG waits for the algorithm
> to be tested; however, CRYPTO_MSG_NEWALG returns early when interrupted.
>
> Everything else in the "crypto user configuration" API has this same bug
> too, i.e. it inappropriately allows operating on larval algorithms
> (though it doesn't look like the other cases can cause a crash).
>
> Fix this by making crypto_alg_match() exclude larval algorithms.
>
> [1] https://lkml.kernel.org/r/[email protected]
> [2] https://github.com/linux-test-project/ltp/blob/20190517/testcases/kernel/crypto/pcrypt_aead01.c
>
> Reported-by: Michal Suchanek <[email protected]>
> Fixes: a38f7907b926 ("crypto: Add userspace configuration API")
> Cc: <[email protected]> # v3.2+
> Cc: Steffen Klassert <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> crypto/crypto_user_base.c | 3 +++
> 1 file changed, 3 insertions(+)
Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, 3 Jul 2019 22:30:57 +0800
Herbert Xu <[email protected]> wrote:
> On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <[email protected]>
> >
> > Michal Suchanek reported [1] that running the pcrypt_aead01 test from
> > LTP [2] in a loop and holding Ctrl-C causes a NULL dereference of
> > alg->cra_users.next in crypto_remove_spawns(), via crypto_del_alg().
> > The test repeatedly uses CRYPTO_MSG_NEWALG and CRYPTO_MSG_DELALG.
> >
> > The crash occurs when the instance that CRYPTO_MSG_DELALG is trying to
> > unregister isn't a real registered algorithm, but rather is a "test
> > larval", which is a special "algorithm" added to the algorithms list
> > while the real algorithm is still being tested. Larvals don't have
> > initialized cra_users, so that causes the crash. Normally pcrypt_aead01
> > doesn't trigger this because CRYPTO_MSG_NEWALG waits for the algorithm
> > to be tested; however, CRYPTO_MSG_NEWALG returns early when interrupted.
> >
Do you have some way to reproduce this reliably?
I suppose you would have to send a signal to the process for the call
to get interrupted, right?
Thanks
Michal