On Tue, 12 May 2020 at 22:31, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Tue, May 12, 2020 at 10:08 PM Eric Biggers <ebiggers@xxxxxxxxxx> 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.
