From: Corentin Labbe <clabbe.montjoie@gmail.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: herbert@gondor.apana.org.au, davem@davemloft.net,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [BUG] crypto: export() overran state buffer on test vector
Date: Fri, 7 Feb 2020 11:46:59 +0100 [thread overview]
Message-ID: <20200207104659.GA10979@Red> (raw)
In-Reply-To: <20200207065719.GA8284@sol.localdomain>
On Thu, Feb 06, 2020 at 10:57:19PM -0800, Eric Biggers wrote:
> On Thu, Feb 06, 2020 at 09:54:42AM +0100, Corentin Labbe wrote:
> > Hello
> >
> > When working on adding hash support on sun8i-ce, I made a simple version which always fallback.
> > but booting it lead to this:
> > [ 52.274278] sun8i-ce 1c15000.crypto: Register sha1
> > [ 52.279286] sun8i-ce 1c15000.crypto: sun8i_hash_crainit statesize is 96
> > [ 52.285933] sun8i-ce 1c15000.crypto: Fallback for sha1-sun8i-ce is sha1-ce
> > [ 52.312423] shash_default_export descsize=104
> > [ 52.316021] alg: ahash: sha1-sun8i-ce export() overran state buffer on test vector 0, cfg=\"import/export\" statesize=96
> > [ 52.333189] sun8i-ce 1c15000.crypto: Register sha224
> > [ 52.338387] sun8i-ce 1c15000.crypto: sun8i_hash_crainit statesize is 104
> > [ 52.345097] sun8i-ce 1c15000.crypto: Fallback for sha224-sun8i-ce is sha224-ce
> > [ 52.371865] shash_default_export descsize=112
> > [ 52.375459] alg: ahash: sha224-sun8i-ce export() overran state buffer on test vector 0, cfg=\"import/export\" statesize=104
> > [ 52.393039] sun8i-ce 1c15000.crypto: Register sha256
> > [ 52.398219] sun8i-ce 1c15000.crypto: sun8i_hash_crainit statesize is 104
> > [ 52.404937] sun8i-ce 1c15000.crypto: Fallback for sha256-sun8i-ce is sha256-ce
> > [ 52.431476] shash_default_export descsize=112
> > [ 52.435073] alg: ahash: sha256-sun8i-ce export() overran state buffer on test vector 0, cfg=\"import/export\" statesize=104
> >
> > For sha1, sha224 and sha256, my driver fail to pass the test.
> > This is due to the fact that export() (and so shash_async_export/shash_default_export) use crypto_shash_descsize() as length but selftest expect it to be statesize.
>
> That doesn't appear to actually be the problem. shash_default_export() does
> assume descsize == statesize, but it's only used when that's the case.
> See shash_prepare_alg().
>
> > Just in case, this is my export code:
> > int sun8i_hash_crainit(struct crypto_tfm *tfm)
> > {
> > struct sun8i_hash_tfm_ctx *op = crypto_tfm_ctx(tfm);
> > struct ahash_alg *alg = __crypto_ahash_alg(tfm->__crt_alg);
> > struct sun8i_ce_alg_template *algt;
> >
> > memset(op, 0, sizeof(struct sun8i_hash_tfm_ctx));
> >
> > crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm), sizeof(struct sun8i_hash_reqctx));
> >
> > op->fallback_tfm = crypto_alloc_ahash(crypto_tfm_alg_name(tfm), 0, CRYPTO_ALG_NEED_FALLBACK);
> > if (IS_ERR(op->fallback_tfm)) {
> > dev_err(algt->ce->dev, "Fallback driver cound no be loaded\n");
> > return PTR_ERR(op->fallback_tfm);
> > }
> > dev_info(op->ce->dev, "%s statesize is %u\n", __func__, algt->alg.hash.halg.statesize);
> > dev_info(op->ce->dev, "Fallback for %s is %s\n",
> > crypto_tfm_alg_driver_name(tfm),
> > crypto_tfm_alg_driver_name(&op->fallback_tfm->base));
> > return 0;
> > }
> >
> > int sun8i_hash_init(struct ahash_request *areq)
> > {
> > struct sun8i_hash_reqctx *rctx = ahash_request_ctx(areq);
> > struct crypto_ahash *tfm = crypto_ahash_reqtfm(areq);
> > struct sun8i_hash_tfm_ctx *tfmctx = crypto_ahash_ctx(tfm);
> >
> > memset(rctx, 0, sizeof(struct sun8i_hash_reqctx));
> >
> > ahash_request_set_tfm(&rctx->fallback_req, tfmctx->fallback_tfm);
> > rctx->fallback_req.base.flags = areq->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP;
> >
> > return crypto_ahash_init(&rctx->fallback_req);
> > }
> >
> > int sun8i_hash_export(struct ahash_request *areq, void *out)
> > {
> > struct sun8i_hash_reqctx *rctx = ahash_request_ctx(areq);
> > struct crypto_ahash *tfm = crypto_ahash_reqtfm(areq);
> > struct sun8i_hash_tfm_ctx *tfmctx = crypto_ahash_ctx(tfm);
> >
> > ahash_request_set_tfm(&rctx->fallback_req, tfmctx->fallback_tfm);
> > rctx->fallback_req.base.flags = areq->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP;
> >
> > return crypto_ahash_export(&rctx->fallback_req, out);
> > }
>
> It seems the actual problem is that you're doing the export using a fallback
> algorithm, which may have a statesize different from the one you're setting.
>
Should not be a problem since all stuff before export is done by the fallback.
> But I'm not sure what you should do here, since the correct statesize can only
> be known when a tfm is allocated, not when the algorithm is registered.
>
> Possibly statesize needs to be made a property of the tfm (struct crypto_ahash
> and crypto_shash) rather than the algorithm (struct hash_alg_common).
>
> But are you sure you actually need a fallback algorithm for any hash algorithms
> in your driver in the first place?
>
My goal is to do like n2-crypto/rk3288crypto/etc..., fallback for init/update/final/finup and only do stuff with digest().
So I have just exactly copied what they do.
Regards
next prev parent reply other threads:[~2020-02-07 10:47 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-06 8:54 [BUG] crypto: export() overran state buffer on test vector Corentin Labbe
2020-02-07 6:57 ` Eric Biggers
2020-02-07 10:46 ` Corentin Labbe [this message]
2020-02-08 8:57 ` Herbert Xu
2020-02-11 19:21 ` Corentin Labbe
2020-02-12 2:06 ` Herbert Xu
2020-02-12 18:57 ` Corentin Labbe
2020-02-13 5:05 ` Herbert Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200207104659.GA10979@Red \
--to=clabbe.montjoie@gmail.com \
--cc=davem@davemloft.net \
--cc=ebiggers@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).