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.
