Re: [PATCH v2 11/11] crypto: skcipher: Remove VLA usage for SKCIPHER_REQUEST_ON_STACK

On Wed, Jun 27, 2018 at 7:36 AM, Herbert Xu [off-list ref] wrote:

On Tue, Jun 26, 2018 at 09:45:09AM -0700, Kees Cook wrote:

quoted

Which are likely to be wrapped together? Should I take this to 512 or
something else?

The situation is similar to ahash.  While they're using the same
skcipher interface, the underlying algorithms must all be
synchronous.  In fact, if they're not then they're buggy.

Therefore it makes no sense to use the general skcipher request
size as a threshold.  You should look at synchronous skcipher
algorithms only.

I might be catching on... so from this list, I should only "count" the
synchronous ones as being wrappable? The skcipher list is actually
pretty short:

crypto/cryptd.c:        crypto_skcipher_set_reqsize(
crypto/cryptd.c-                tfm, sizeof(struct
cryptd_skcipher_request_ctx));

The above is, AIUI, unwrapped, so I only need to count sizeof(struct
cryptd_skcipher_request_ctx)?

These are "simple" wrappers:

crypto/lrw.c:   crypto_skcipher_set_reqsize(tfm,
crypto_skcipher_reqsize(cipher) +
crypto/lrw.c-                                    sizeof(struct rctx));

crypto/simd.c-  reqsize = sizeof(struct skcipher_request);
crypto/simd.c-  reqsize += crypto_skcipher_reqsize(&cryptd_tfm->base);
crypto/simd.c:  crypto_skcipher_set_reqsize(tfm, reqsize);

crypto/xts.c:   crypto_skcipher_set_reqsize(tfm,
crypto_skcipher_reqsize(child) +
crypto/xts.c-                                    sizeof(struct rctx));

But what are the "legitimate" existing crypto_skcipher_reqsize() values here?

These are "complex" wrappers, with cts even adding blocksize to the mix...

crypto/ctr.c-   align = crypto_skcipher_alignmask(tfm);
crypto/ctr.c-   align &= ~(crypto_tfm_ctx_alignment() - 1);
crypto/ctr.c-   reqsize = align + sizeof(struct crypto_rfc3686_req_ctx) +
crypto/ctr.c-             crypto_skcipher_reqsize(cipher);
crypto/ctr.c:   crypto_skcipher_set_reqsize(tfm, reqsize);

crypto/cts.c-   align = crypto_skcipher_alignmask(tfm);
crypto/cts.c-   bsize = crypto_skcipher_blocksize(cipher);
crypto/cts.c-   reqsize = ALIGN(sizeof(struct crypto_cts_reqctx) +
crypto/cts.c-                   crypto_skcipher_reqsize(cipher),
crypto/cts.c-                   crypto_tfm_ctx_alignment()) +
crypto/cts.c-             (align & ~(crypto_tfm_ctx_alignment() - 1)) + bsize;
crypto/cts.c-
crypto/cts.c:   crypto_skcipher_set_reqsize(tfm, reqsize);

What values might be expected here? It seems the entire blocksize
needs to be included as well...

-Kees

-- 
Kees Cook
Pixel Security