Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180627222749.xa5333caq5vaa7st@gondor.apana.org.au>
References: <20180625211026.15819-1-keescook@chromium.org> <20180625211026.15819-12-keescook@chromium.org>
 <20180626092041.mxfg4lxcvxfivzc2@gondor.apana.org.au> <CAGXu5jKYb-0NNvOcN79hzqU7O6vCFhMTBDGBL8Knuoc-YHamPg@mail.gmail.com>
 <20180627143622.ntksjxsymo4yw6dz@gondor.apana.org.au> <CAGXu5jJ2Z=YP+fNsvy+6a_BpC1ibZf6tOzZuPHk=Qx1qNVvOZw@mail.gmail.com>
 <20180627222749.xa5333caq5vaa7st@gondor.apana.org.au>
From:   Kees Cook <keescook@chromium.org>
Date:   Wed, 27 Jun 2018 17:10:05 -0700
Message-ID: <CAGXu5jKas=OyJPsrMthxjEykG=FkHDW84dO4bKGpj-50JjUYwQ@mail.gmail.com>
Subject: Re: [PATCH v2 11/11] crypto: skcipher: Remove VLA usage for SKCIPHER_REQUEST_ON_STACK
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     "David S. Miller" <davem@davemloft.net>,
        linux-crypto <linux-crypto@vger.kernel.org>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Eric Biggers <ebiggers@google.com>,
        Alasdair Kergon <agk@redhat.com>,
        Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
        Lars Persson <larper@axis.com>,
        Mike Snitzer <snitzer@redhat.com>,
        Rabin Vincent <rabinv@axis.com>,
        Tim Chen <tim.c.chen@linux.intel.com>, qat-linux@intel.com,
        dm-devel@redhat.com, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jun 27, 2018 at 3:27 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Wed, Jun 27, 2018 at 11:31:09AM -0700, Kees Cook wrote:
>> crypto/lrw.c:   crypto_skcipher_set_reqsize(tfm,
>> crypto_skcipher_reqsize(cipher) +
>> crypto/lrw.c-                                    sizeof(struct rctx));
>> ...
>> 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...
>
> But otherwise yes these are the ones that count.

In both cases here, what is "cipher"? i.e. what ciphers could lrw be
wrapping, and what ciphers could cts be wrapping, so that I can
examine the blocksizes, etc?

FWIW, looking at the non-ASYNC wrappers, I see only:

crypto/ctr.c
crypto/cts.c
crypto/lrw.c
crypto/xts.c
drivers/crypto/sunxi-ss/sun4i-ss-cipher.c

Building in all crypto things and running tcrypt with an instrumented
crypto_skcipher_set_reqsize, I see:

# dmesg | grep skcipher_set_req | cut -c16- | sort -u | sort +1 -n
crypto_skcipher_set_reqsize: 8
crypto_skcipher_set_reqsize: 88
crypto_skcipher_set_reqsize: 184
crypto_skcipher_set_reqsize: 256
crypto_skcipher_set_reqsize: 472

The 472 maps to lrw with its 384 struct rctx:

[  553.843884] tcrypt: testing lrw(twofish)
[  553.844479] crypto_skcipher_set_reqsize: 8
[  553.845076] crypto_skcipher_set_reqsize: 88
[  553.845658] crypto_skcipher_set_reqsize: 472

[  553.860578] tcrypt: testing lrw(serpent)
[  553.861349] crypto_skcipher_set_reqsize: 8
[  553.861960] crypto_skcipher_set_reqsize: 88
[  553.862534] crypto_skcipher_set_reqsize: 472

[  553.871676] tcrypt: testing lrw(aes)
[  553.872398] crypto_skcipher_set_reqsize: 8
[  553.873002] crypto_skcipher_set_reqsize: 88
[  553.873574] crypto_skcipher_set_reqsize: 472

[  553.957282] tcrypt: testing lrw(cast6)
[  553.958098] crypto_skcipher_set_reqsize: 8
[  553.958691] crypto_skcipher_set_reqsize: 88
[  553.959311] crypto_skcipher_set_reqsize: 472

[  553.982514] tcrypt: testing lrw(camellia)
[  553.983308] crypto_skcipher_set_reqsize: 8
[  553.983907] crypto_skcipher_set_reqsize: 88
[  553.984470] crypto_skcipher_set_reqsize: 472

And while I'm using tcrypt, ahash shows:

44
124
336
368
528
536
568
616
648
728
808

The largest seems to be sha512:

[  553.883440] tcrypt: testing sha512
[  553.884179] sha512_mb: crypto_ahash_set_reqsize: 528
[  553.884904] crypto_ahash_set_reqsize: 728
[  553.885449] sha512_mb: crypto_ahash_set_reqsize: 808

So ... should I use 472 for skcipher and 808 for ahash?

-Kees

-- 
Kees Cook
Pixel Security