linux-kernel - Re: linux-next: manual merge of the sound-asoc tree with the crypto tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 May 2020 22:31:27 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Mark Brown <broonie@...nel.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Liam Girdwood <lgirdwood@...il.com>,
        Linux Crypto List <linux-crypto@...r.kernel.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: linux-next: manual merge of the sound-asoc tree with the crypto tree

On Tue, May 12, 2020 at 10:08 PM Eric Biggers <ebiggers@...nel.org> wrote:
> On Tue, May 12, 2020 at 06:08:01PM +0100, Mark Brown wrote:
>
> For later: if SHASH_DESC_ON_STACK is causing problems, we really ought to find a
> better solution, since lots of users are using this macro.  A version of
> crypto_shash_tfm_digest() that falls back to heap allocation if the descsize is
> too large would be possible, but that wouldn't fully solve the problem since
> some users do incremental hashing.

It's hard to know how many of the users of SHASH_DESC_ON_STACK() are
likely to cause problems, as multiple factors are involved:

- this one triggered the warning because it was on the stack of a function
  that got inlined into another that has other large variables. Whether it
  got inlined makes little difference to the stack usage, but does make a
  difference to warning about it.

- generally the structure is larger than we like it, especially on architectures
  with 128 byte CRYPTO_MINALIGN like ARM. This actually got worse
  because of b68a7ec1e9a3 ("crypto: hash - Remove VLA usage"), as
  the stack usage is now always the maximum of all hashes where it used
  to be specific to the hash that was actually used and could be smaller

- the specific instance in calculate_sha256() feels a bit silly, as this
  function allocates a tfm and a descriptor, runs the digest and then
  frees both again. I don't know how common this pattern is, but
  it seems a higher-level abstraction might be helpful anyway.

      Arnd