crypto: export() overran state buffer on test vector

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.

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);                   
}

Quoted 20 lines · Corentin Labbe

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

Quoted 2 lines

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().

Quoted 43 lines

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);

Quoted 5 lines

        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.

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.

Quoted 3 lines · Corentin Labbe

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.

n2 at least is totally broken wrt import/export. The other ones would work provided that the fallback have the same statesize as the generic sha implementations.

Are you not using the standard state sizes?

This should probably be switched over to lib/crypto or at least shash.

Do you mean that I should abandon ahash as a fallback ?

Quoted 2 lines · Corentin Labbe

Do you mean that I should abandon ahash as a fallback ?

Perhaps switching to shash is not as straightforward because of SG handling. But if you're getting problems with statesize perhaps at least force a sync ahash algorithm might be a good idea.

I just found the problem, it happen with ARM CE which has a bigger state_size (sha256_ce_state) of 4bytes.

Since my driver didnt use statesize at all, I detect this on cra_init() like this:
if (algt->alg.hash.halg.statesize < crypto_ahash_statesize(op->fallback_tfm))

	algt->alg.hash.halg.statesize = crypto_ahash_statesize(op->fallback_tfm);