On Thu, Sep 6, 2018 at 8:42 PM, Herbert Xu [off-list ref] wrote:

On Thu, Sep 06, 2018 at 03:58:52PM -0700, Kees Cook wrote:

quoted

@@ -437,6 +442,12 @@ static inline struct crypto_skcipher *crypto_skcipher_reqtfm_check(
 {
      struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);

+     if (req->__onstack) {
+             if (WARN_ON(crypto_skcipher_alg(tfm)->base.cra_flags &
+                             CRYPTO_ALG_ASYNC))
+                     return ERR_PTR(-EINVAL);
+     }

Sorry but I don't like imposing a run-time check on everybody when
stack-based requests are the odd ones out.  If we're going to make
this a run-time check (I'd much prefer a compile-time check, but I
understand that this may involve too much churn), then please do it
for stack-based request users only.

I'll continue to investigate alternatives, but I wanted to point out
that the struct change actually fills an existing padding byte (so no
change in memory usage) and marking this as an unlikely() test means
it wouldn't even be measurable due to the branch predictor (so no
change in speed). encrypt/decrypt entry is a tiny tiny fraction of the
actual work done during encryption/decryption, etc.

-Kees

-- 
Kees Cook
Pixel Security