On Mon, May 22, 2017 at 08:55:31PM +0200, Mike Belopuhov wrote:
> On Mon, May 22, 2017 at 20:20 +0200, Sebastien Marie wrote:
> > On Mon, May 22, 2017 at 07:32:07PM +0200, Mike Belopuhov wrote:
> > > >
> > > > With the last commit to revert AES_XTS to rijndael, I pushed it on
> > > > top of the tested tree (7 days old). The hibernate/resume works again.
> > > >
> > > > It makes it to confirm the problem is related to the switch to
> > > > constant-time-aes in the context of full-disk-encryption.
> > > >
> > >
> > > Thanks for verifying this. I've looked through the sr_hibernate_io
> > > (that's hib->io_func) but couldn't find anything wrong with it. The
> > > only thing that springs to mind is that AES_CTX and therefore the
> > > XTS context (aes_xts_ctx) is larger and requires more stack space.
> > > Though I can't see what might be affected by that.
> >
> > I quickly check the difference of struct size and come back with some
> > idea to test.
> >
> > - aes_xts_ctx based on AES_CTX has sizeof()=1464
> > - aes_xts_ctx based on rijndael_ctx has sizeof()=992
> >
> >
> > do you think just adding padding of 472 (1464-992) at end of `struct
> > aes_xts_ctx' (rijndael_ctx version) would be enough to test it ?
> >
> > Something like:
> >
> > diff --git a/sys/crypto/xform.c b/sys/crypto/xform.c
> > index 71e173b44..9775d030c 100644
> > --- a/sys/crypto/xform.c
> > +++ b/sys/crypto/xform.c
> > @@ -125,6 +125,7 @@ struct aes_xts_ctx {
> > rijndael_ctx key1;
> > rijndael_ctx key2;
> > u_int8_t tweak[AES_XTS_BLOCKSIZE];
> > + u_int8_t _pad[472];
> > };
> >
> > /* Helper */
> >
>
> Good question. I would try both in the end of the structure and in
> the beginning, in attempt to at least rule out stack corruption.
>
> >
> > if the hibernate/resume process is able to complete with the padded
> > version, we could exclude the struct size of the equation. and if the
> > process fail, it means the size matters.
> >
>
> I would agree with this.
>
> > any additional thing to do for testing padding (initialisation or
> > something else) ? you know better than me this subsystem :)
> >
>
> Nothing springs to mind, but I didn't have time to investigate
> any further yet.
>
Is the new AES implementation side effect free? It cannot touch any memory
outside the scratch page used by the sr crypto suspend routine.
-ml
