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] [day] [month] [year] [list]

Date:   Thu, 14 May 2020 15:21:12 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Eric Biggers <ebiggers@...nel.org>,
        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, 12 May 2020 at 22:31, Arnd Bergmann <arnd@...db.de> wrote:
>
> 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.
>

We are trying to move to crypto library interfaces for non-performance
critical uses of hashes where the algorithm is known at compile time,
and this is a good example of that pattern.

IOW, this code should just call the sha256_init/update/final routines directly.

I'll send out a patch.