* [PATCH] crypto: user - prevent operating on larval algorithms [not found] <20190701153154.1569c2dc@kitsune.suse.cz> @ 2019-07-02 21:17 ` Eric Biggers 2019-07-03 14:30 ` Herbert Xu 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2019-07-02 21:17 UTC (permalink / raw) To: linux-crypto, Herbert Xu Cc: chetjain, David S . Miller, linux-kernel, Michal Suchanek, stable, Steffen Klassert From: Eric Biggers <ebiggers@google.com> 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/20190625071624.27039-1-msuchanek@suse.de [2] https://github.com/linux-test-project/ltp/blob/20190517/testcases/kernel/crypto/pcrypt_aead01.c Reported-by: Michal Suchanek <msuchanek@suse.de> Fixes: a38f7907b926 ("crypto: Add userspace configuration API") Cc: <stable@vger.kernel.org> # v3.2+ Cc: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Eric Biggers <ebiggers@google.com> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: user - prevent operating on larval algorithms 2019-07-02 21:17 ` [PATCH] crypto: user - prevent operating on larval algorithms Eric Biggers @ 2019-07-03 14:30 ` Herbert Xu 2019-07-03 20:21 ` Michal Suchánek 0 siblings, 1 reply; 5+ messages in thread From: Herbert Xu @ 2019-07-03 14:30 UTC (permalink / raw) To: Eric Biggers Cc: linux-crypto, chetjain, David S . Miller, linux-kernel, Michal Suchanek, stable, Steffen Klassert On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > 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/20190625071624.27039-1-msuchanek@suse.de > [2] https://github.com/linux-test-project/ltp/blob/20190517/testcases/kernel/crypto/pcrypt_aead01.c > > Reported-by: Michal Suchanek <msuchanek@suse.de> > Fixes: a38f7907b926 ("crypto: Add userspace configuration API") > Cc: <stable@vger.kernel.org> # v3.2+ > Cc: Steffen Klassert <steffen.klassert@secunet.com> > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > crypto/crypto_user_base.c | 3 +++ > 1 file changed, 3 insertions(+) Patch applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: user - prevent operating on larval algorithms 2019-07-03 14:30 ` Herbert Xu @ 2019-07-03 20:21 ` Michal Suchánek 2019-07-03 20:31 ` Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: Michal Suchánek @ 2019-07-03 20:21 UTC (permalink / raw) To: Herbert Xu Cc: Eric Biggers, linux-crypto, chetjain, David S . Miller, linux-kernel, stable, Steffen Klassert On Wed, 3 Jul 2019 22:30:57 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote: > On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: user - prevent operating on larval algorithms 2019-07-03 20:21 ` Michal Suchánek @ 2019-07-03 20:31 ` Eric Biggers 2019-07-03 21:10 ` Michal Suchánek 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2019-07-03 20:31 UTC (permalink / raw) To: Michal Suchánek Cc: Herbert Xu, linux-crypto, chetjain, David S . Miller, linux-kernel, stable, Steffen Klassert Hi Michal, On Wed, Jul 03, 2019 at 10:21:08PM +0200, Michal Suchánek wrote: > On Wed, 3 Jul 2019 22:30:57 +0800 > Herbert Xu <herbert@gondor.apana.org.au> wrote: > > > On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote: > > > From: Eric Biggers <ebiggers@google.com> > > > > > > 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? > It reproduced pretty reliably for me with what you suggested. Just typing in terminal: while true; do pcrypt_aead01; done and then holding Ctrl-C. If I have time I'll try writing an LTP test that specifically reproduces it. Yes, it would involve sending a signal to a thread or process that's executing CRYPTO_MSG_NEWALG (unless I find a better way). - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: user - prevent operating on larval algorithms 2019-07-03 20:31 ` Eric Biggers @ 2019-07-03 21:10 ` Michal Suchánek 0 siblings, 0 replies; 5+ messages in thread From: Michal Suchánek @ 2019-07-03 21:10 UTC (permalink / raw) To: Eric Biggers Cc: Herbert Xu, linux-crypto, chetjain, David S . Miller, linux-kernel, stable, Steffen Klassert On Wed, 3 Jul 2019 13:31:29 -0700 Eric Biggers <ebiggers@kernel.org> wrote: > Hi Michal, > > On Wed, Jul 03, 2019 at 10:21:08PM +0200, Michal Suchánek wrote: > > On Wed, 3 Jul 2019 22:30:57 +0800 > > Herbert Xu <herbert@gondor.apana.org.au> wrote: > > > > > On Tue, Jul 02, 2019 at 02:17:00PM -0700, Eric Biggers wrote: > > > > From: Eric Biggers <ebiggers@google.com> > > > > > > > > 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? > > > > It reproduced pretty reliably for me with what you suggested. Just typing in > terminal: > > while true; do pcrypt_aead01; done > > and then holding Ctrl-C. > > If I have time I'll try writing an LTP test that specifically reproduces it. > Yes, it would involve sending a signal to a thread or process that's executing > CRYPTO_MSG_NEWALG (unless I find a better way). Maybe it is possible to just send the remove message without waiting for the ack on add. Thanks Michal ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-03 21:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190701153154.1569c2dc@kitsune.suse.cz>
2019-07-02 21:17 ` [PATCH] crypto: user - prevent operating on larval algorithms Eric Biggers
2019-07-03 14:30 ` Herbert Xu
2019-07-03 20:21 ` Michal Suchánek
2019-07-03 20:31 ` Eric Biggers
2019-07-03 21:10 ` Michal Suchánek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).