On Sat, Nov 03, 2018 at 03:35:04PM -0700, Eric Biggers wrote:
> [+cla...@baylibre.com]
> 
> Hi Corentin, I think this is a bug in the new crypto statistics feature.  In 
> the
> skcipher_decrypt case the code is (but this applies elsewhere too!):
> 
> static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
>                                               int ret, struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
>       if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
>               atomic_inc(&alg->cipher_err_cnt);
>       } else {
>               atomic_inc(&alg->decrypt_cnt);
>               atomic64_add(req->cryptlen, &alg->decrypt_tlen);
>       }
> #endif
> }
> 
> static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
> {
>       struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
>       int ret;
> 
>       if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
>               ret = -ENOKEY;
>       else
>               ret = tfm->decrypt(req);
>       crypto_stat_skcipher_decrypt(req, ret, tfm->base.__crt_alg);
>       return ret;
> }
> 
> The bug is the request may be issued asynchronously (as indicated by 
> EINPROGRESS
> or EBUSY) being returned, and the stats are updated afterwards.  But by that
> time, the request's completion function may have already run, and the request
> structure may have already been freed.
> 
> In theory, I think the algorithm could have even been unregistered as well.
> Therefore, it's only safe to update the stats either *before* calling
> tfm->decrypt(), or afterwards if the error code was not EINPROGRESS or EBUSY.

Hello

I can store "len" and alg for later use, this will fix a part of the problem.

For the fact that algorithm could be unregistred, I think it cannot happen 
since at least the tfm running this crypto_skcipher_decrypt/othersamefunction 
still exists and that it is(should be) impossible to unregister an alg with 
still existing tfm which uses it.
But that needs to be verified.

Regards

Reply via email to