| Message ID | E1id3Te-00077s-Ux@gondobar (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: shash - Enforce descsize limit in init_tfm | expand |
On Fri, Dec 06, 2019 at 10:36:18AM +0800, Herbert Xu wrote: > The shash interface supports a dynamic descsize field because of > the presence of fallbacks (it's just padlock-sha actually, perhaps > we can remove it one day). As it is the API does not verify the > setting of descsize at all. It is up to the individual algorithms > to ensure that descsize does not exceed the specified maximum value > of HASH_MAX_DESCSIZE (going above would cause stack corruption). > > In order to allow the API to impose this limit directly, this patch > adds init_tfm/exit_tfm hooks to the shash_alg structure. We can > then verify the descsize setting in the API directly. > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > --- > > crypto/shash.c | 25 +++++++++++++++++++++++++ > include/crypto/hash.h | 14 ++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/crypto/shash.c b/crypto/shash.c > index e83c5124f6eb..40628712feec 100644 > --- a/crypto/shash.c > +++ b/crypto/shash.c > @@ -386,15 +386,40 @@ int crypto_init_shash_ops_async(struct crypto_tfm *tfm) > return 0; > } > > +static void crypto_shash_exit_tfm(struct crypto_tfm *tfm) > +{ > + struct crypto_shash *hash = __crypto_shash_cast(tfm); > + struct shash_alg *alg = crypto_shash_alg(hash); > + > + alg->exit_tfm(hash); > +} > + > static int crypto_shash_init_tfm(struct crypto_tfm *tfm) > { > struct crypto_shash *hash = __crypto_shash_cast(tfm); > struct shash_alg *alg = crypto_shash_alg(hash); > + int err; > > hash->descsize = alg->descsize; > > shash_set_needkey(hash, alg); > > + if (alg->exit_tfm) > + tfm->exit = crypto_shash_exit_tfm; > + > + if (!alg->init_tfm) > + return 0; > + > + err = alg->init_tfm(hash); > + if (err) > + return err; > + > + if (hash->descsize > HASH_MAX_DESCSIZE) { Use WARN_ON_ONCE() here? If HASH_MAX_DESCSIZE is too low for some case, it's a bug that needs to be fixed. > + * @init_tfm: Initialize the cryptographic transformation object. > + * This function is used to initialize the cryptographic > + * transformation object. This function is called only > + * once at the instantiation time, right after the > + * transformation context was allocated. In case the > + * cryptographic hardware has some special requirements > + * which need to be handled by software, this function > + * shall check for the precise requirement of the > + * transformation and put any software fallbacks in place. The second sentence can be removed, since it's redundant with the first. - Eric
diff --git a/crypto/shash.c b/crypto/shash.c index e83c5124f6eb..40628712feec 100644 --- a/crypto/shash.c +++ b/crypto/shash.c @@ -386,15 +386,40 @@ int crypto_init_shash_ops_async(struct crypto_tfm *tfm) return 0; } +static void crypto_shash_exit_tfm(struct crypto_tfm *tfm) +{ + struct crypto_shash *hash = __crypto_shash_cast(tfm); + struct shash_alg *alg = crypto_shash_alg(hash); + + alg->exit_tfm(hash); +} + static int crypto_shash_init_tfm(struct crypto_tfm *tfm) { struct crypto_shash *hash = __crypto_shash_cast(tfm); struct shash_alg *alg = crypto_shash_alg(hash); + int err; hash->descsize = alg->descsize; shash_set_needkey(hash, alg); + if (alg->exit_tfm) + tfm->exit = crypto_shash_exit_tfm; + + if (!alg->init_tfm) + return 0; + + err = alg->init_tfm(hash); + if (err) + return err; + + if (hash->descsize > HASH_MAX_DESCSIZE) { + if (alg->exit_tfm) + alg->exit_tfm(hash); + return -EINVAL; + } + return 0; } diff --git a/include/crypto/hash.h b/include/crypto/hash.h index fe7f73bad1e2..ba1435b38266 100644 --- a/include/crypto/hash.h +++ b/include/crypto/hash.h @@ -169,6 +169,18 @@ struct shash_desc { * @export: see struct ahash_alg * @import: see struct ahash_alg * @setkey: see struct ahash_alg + * @init_tfm: Initialize the cryptographic transformation object. + * This function is used to initialize the cryptographic + * transformation object. This function is called only + * once at the instantiation time, right after the + * transformation context was allocated. In case the + * cryptographic hardware has some special requirements + * which need to be handled by software, this function + * shall check for the precise requirement of the + * transformation and put any software fallbacks in place. + * @exit_tfm: Deinitialize the cryptographic transformation object. + * This is a counterpart to @init_tfm, used to remove + * various changes set in @init_tfm. * @digestsize: see struct ahash_alg * @statesize: see struct ahash_alg * @descsize: Size of the operational state for the message digest. This state @@ -189,6 +201,8 @@ struct shash_alg { int (*import)(struct shash_desc *desc, const void *in); int (*setkey)(struct crypto_shash *tfm, const u8 *key, unsigned int keylen); + int (*init_tfm)(struct crypto_shash *tfm); + void (*exit_tfm)(struct crypto_shash *tfm); unsigned int descsize;
The shash interface supports a dynamic descsize field because of the presence of fallbacks (it's just padlock-sha actually, perhaps we can remove it one day). As it is the API does not verify the setting of descsize at all. It is up to the individual algorithms to ensure that descsize does not exceed the specified maximum value of HASH_MAX_DESCSIZE (going above would cause stack corruption). In order to allow the API to impose this limit directly, this patch adds init_tfm/exit_tfm hooks to the shash_alg structure. We can then verify the descsize setting in the API directly. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> --- crypto/shash.c | 25 +++++++++++++++++++++++++ include/crypto/hash.h | 14 ++++++++++++++ 2 files changed, 39 insertions(+)