Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7

[Marcelo Cerri <marcelo.cerri@canonical.com>]

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

Hi Herbert,

On Wed, Sep 28, 2016 at 10:45:49AM +0800, Herbert Xu wrote:
> On Tue, Sep 27, 2016 at 04:46:44PM -0300, Marcelo Cerri wrote:
> >
> > Can you check if the problem occurs with this patch?
>
> In light of the fact that padlock-sha is the correct example
> to follow, you only need to add one line to the init_tfm fucntion
> to update the descsize based on that of the fallback.
>

Not sure if I'm missing something here but p8_ghash already does that:

 56 static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
 57 {
 ...
 83         shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx)
 84             + crypto_shash_descsize(fallback);
 85
 86         return 0;
 87 }

I think the problem here is that crypto_ahash_statesize uses the
statesize value (that is initialized with the descsize value from
shash_alg) instead of using the value from the tfm that was updated.

For padlock_sha1, it uses the value from alg->statesize since it defines
import and export functions. It doesn't make much difference if it uses
the value from descsize or statesize here, what really matter is that
it uses the value defined in struct shash_alg and not from the tfm.

The big difference between p8_ghash and padlock_sha1 is that
padlock_sha1 defines alg->statesize as sizeof(struct sha1_state), which
is the descsize value used by sha1_generic. This probably works but
it's also wrong because the padlock_sha1 driver does not ensures that
sha1_generic is always used.

So, one solution is to hardcode ghash-generic as the fallback algorithm
and update the descsize direct in its shash_alg structure. There's only
one problem with that. ghash-generic desc type (struct ghash_desc_ctx)
is not available for vmx_crypto at compile type, the type is defined
directly in crypto/ghash-generic.c. That's the reason I added a function
to get the fallback desc size at runtime in the patch I wrote as a prove
of concept.

--
Regards,
Marcelo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]