Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Eric Biggers <ebiggers@google.com>]

On Sat, Oct 01, 2016 at 06:03:31PM +0200, Richard Weinberger wrote:
> Eric,
> 
> On Fri, Sep 30, 2016 at 7:58 PM, Eric Biggers <ebiggers@google.com> wrote:
> > The XTS tweak (or IV) was initialized differently on little endian and
> > big endian systems.  Because the ciphertext depends on the XTS tweak, it
> > was not possible to use an encrypted filesystem created by a little
> > endian system on a big endian system and vice versa, even if they shared
> > the same PAGE_SIZE.  Fix this by always using little endian.
> >
> > This will break hypothetical big endian users of ext4 or f2fs
> > encryption.  However, all users we are aware of are little endian, and
> > it's believed that "real" big endian users are unlikely to exist yet.
> > So this might as well be fixed now before it's too late.
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  fs/crypto/crypto.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 61057b7d..98f87fe 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -151,7 +151,10 @@ static int do_page_crypto(struct inode *inode,
> >                         struct page *src_page, struct page *dest_page,
> >                         gfp_t gfp_flags)
> >  {
> > -       u8 xts_tweak[FS_XTS_TWEAK_SIZE];
> > +       struct {
> > +               __le64 index;
> > +               u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
> > +       } xts_tweak;
> 
> While we are here, wouldn't it make sense to rename the variable to "iv"?
> In aes-xts mode the IV is used as tweak. But it is still an IV and passed
> as IV parameter to the crypto API.
> 
> Especially when other cipher modes are used this is confusing.
> 

Good idea --- I agree that "iv" is a better name, so as to not tie the code to
XTS specifically.  But I think the renaming should be a separate patch.

Also, currently this code *is* only supposed to be used for XTS.  There's a bug
where a specially crafted filesystem can cause this code path to be entered with
CTS, but I have a patch pending in the ext4 tree to fix that.

Eric