projects / thirdparty / kernel / stable-queue.git / blame
| Commit | Line | Data |
|---|---|---|
| 128027b5 GKH |
1 | From b1f6b4bf416b49f00f3abc49c639371cdecaaad1 Mon Sep 17 00:00:00 2001 |
| 2 | From: Eric Biggers <ebiggers@google.com> | |
| 3 | Date: Sun, 6 Jan 2019 18:47:43 -0800 | |
| 4 | Subject: crypto: skcipher - set CRYPTO_TFM_NEED_KEY if ->setkey() fails | |
| 5 | ||
| 6 | From: Eric Biggers <ebiggers@google.com> | |
| 7 | ||
| 8 | commit b1f6b4bf416b49f00f3abc49c639371cdecaaad1 upstream. | |
| 9 | ||
| 10 | Some algorithms have a ->setkey() method that is not atomic, in the | |
| 11 | sense that setting a key can fail after changes were already made to the | |
| 12 | tfm context. In this case, if a key was already set the tfm can end up | |
| 13 | in a state that corresponds to neither the old key nor the new key. | |
| 14 | ||
| 15 | For example, in lrw.c, if gf128mul_init_64k_bbe() fails due to lack of | |
| 16 | memory, then priv::table will be left NULL. After that, encryption with | |
| 17 | that tfm will cause a NULL pointer dereference. | |
| 18 | ||
| 19 | It's not feasible to make all ->setkey() methods atomic, especially ones | |
| 20 | that have to key multiple sub-tfms. Therefore, make the crypto API set | |
| 21 | CRYPTO_TFM_NEED_KEY if ->setkey() fails and the algorithm requires a | |
| 22 | key, to prevent the tfm from being used until a new key is set. | |
| 23 | ||
| 24 | [Cc stable mainly because when introducing the NEED_KEY flag I changed | |
| 25 | AF_ALG to rely on it; and unlike in-kernel crypto API users, AF_ALG | |
| 26 | previously didn't have this problem. So these "incompletely keyed" | |
| 27 | states became theoretically accessible via AF_ALG -- though, the | |
| 28 | opportunities for causing real mischief seem pretty limited.] | |
| 29 | ||
| 30 | Fixes: f8d33fac8480 ("crypto: skcipher - prevent using skciphers without setting key") | |
| 31 | Cc: <stable@vger.kernel.org> # v4.16+ | |
| 32 | Signed-off-by: Eric Biggers <ebiggers@google.com> | |
| 33 | Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> | |
| 34 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
| 35 | ||
| 36 | --- | |
| 37 | crypto/skcipher.c | 27 ++++++++++++++++++--------- | |
| 38 | 1 file changed, 18 insertions(+), 9 deletions(-) | |
| 39 | ||
| 40 | --- a/crypto/skcipher.c | |
| 41 | +++ b/crypto/skcipher.c | |
| 42 | @@ -584,6 +584,12 @@ static unsigned int crypto_skcipher_exts | |
| 43 | return crypto_alg_extsize(alg); | |
| 44 | } | |
| 45 | ||
| 46 | +static void skcipher_set_needkey(struct crypto_skcipher *tfm) | |
| 47 | +{ | |
| 48 | + if (tfm->keysize) | |
| 49 | + crypto_skcipher_set_flags(tfm, CRYPTO_TFM_NEED_KEY); | |
| 50 | +} | |
| 51 | + | |
| 52 | static int skcipher_setkey_blkcipher(struct crypto_skcipher *tfm, | |
| 53 | const u8 *key, unsigned int keylen) | |
| 54 | { | |
| 55 | @@ -597,8 +603,10 @@ static int skcipher_setkey_blkcipher(str | |
| 56 | err = crypto_blkcipher_setkey(blkcipher, key, keylen); | |
| 57 | crypto_skcipher_set_flags(tfm, crypto_blkcipher_get_flags(blkcipher) & | |
| 58 | CRYPTO_TFM_RES_MASK); | |
| 59 | - if (err) | |
| 60 | + if (unlikely(err)) { | |
| 61 | + skcipher_set_needkey(tfm); | |
| 62 | return err; | |
| 63 | + } | |
| 64 | ||
| 65 | crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY); | |
| 66 | return 0; | |
| 67 | @@ -676,8 +684,7 @@ static int crypto_init_skcipher_ops_blkc | |
| 68 | skcipher->ivsize = crypto_blkcipher_ivsize(blkcipher); | |
| 69 | skcipher->keysize = calg->cra_blkcipher.max_keysize; | |
| 70 | ||
| 71 | - if (skcipher->keysize) | |
| 72 | - crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY); | |
| 73 | + skcipher_set_needkey(skcipher); | |
| 74 | ||
| 75 | return 0; | |
| 76 | } | |
| 77 | @@ -697,8 +704,10 @@ static int skcipher_setkey_ablkcipher(st | |
| 78 | crypto_skcipher_set_flags(tfm, | |
| 79 | crypto_ablkcipher_get_flags(ablkcipher) & | |
| 80 | CRYPTO_TFM_RES_MASK); | |
| 81 | - if (err) | |
| 82 | + if (unlikely(err)) { | |
| 83 | + skcipher_set_needkey(tfm); | |
| 84 | return err; | |
| 85 | + } | |
| 86 | ||
| 87 | crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY); | |
| 88 | return 0; | |
| 89 | @@ -775,8 +784,7 @@ static int crypto_init_skcipher_ops_ablk | |
| 90 | sizeof(struct ablkcipher_request); | |
| 91 | skcipher->keysize = calg->cra_ablkcipher.max_keysize; | |
| 92 | ||
| 93 | - if (skcipher->keysize) | |
| 94 | - crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY); | |
| 95 | + skcipher_set_needkey(skcipher); | |
| 96 | ||
| 97 | return 0; | |
| 98 | } | |
| 99 | @@ -819,8 +827,10 @@ static int skcipher_setkey(struct crypto | |
| 100 | else | |
| 101 | err = cipher->setkey(tfm, key, keylen); | |
| 102 | ||
| 103 | - if (err) | |
| 104 | + if (unlikely(err)) { | |
| 105 | + skcipher_set_needkey(tfm); | |
| 106 | return err; | |
| 107 | + } | |
| 108 | ||
| 109 | crypto_skcipher_clear_flags(tfm, CRYPTO_TFM_NEED_KEY); | |
| 110 | return 0; | |
| 111 | @@ -852,8 +862,7 @@ static int crypto_skcipher_init_tfm(stru | |
| 112 | skcipher->ivsize = alg->ivsize; | |
| 113 | skcipher->keysize = alg->max_keysize; | |
| 114 | ||
| 115 | - if (skcipher->keysize) | |
| 116 | - crypto_skcipher_set_flags(skcipher, CRYPTO_TFM_NEED_KEY); | |
| 117 | + skcipher_set_needkey(skcipher); | |
| 118 | ||
| 119 | if (alg->exit) | |
| 120 | skcipher->base.exit = crypto_skcipher_exit_tfm; |