> -----Original Message----- > From: Ondrej Mosnáček <omosnacek@xxxxxxxxx> > Sent: Wednesday, August 7, 2019 10:33 PM > To: Milan Broz <gmazyland@xxxxxxxxx> > Cc: Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>; Pascal van Leeuwen > <pascalvanl@xxxxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx; rsnel@xxxxxxxxxxxxxxx; > herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx > Subject: Re: [PATCH] crypto: xts - Add support for Cipher Text Stealing > > st 7. 8. 2019 o 19:44 Milan Broz <gmazyland@xxxxxxxxx> napísal(a): > > On 07/08/2019 17:13, Pascal Van Leeuwen wrote: > > >>>> Seems there is no mistake in your code, it is some bug in aesni_intel implementation. > > >>>> If I disable this module, it works as expected (with aes generic and aes_i586). > > >>>> > > >>> That's odd though, considering there is a dedicated xts-aes-ni implementation, > > >>> i.e. I would not expect that to end up at the generic xts wrapper at all? > > >> > > >> Note it is 32bit system, AESNI XTS is under #ifdef CONFIG_X86_64 so it is not used. > > >> > > > Ok, so I guess no one bothered to make an optimized XTS version for i386. > > > I quickly browsed through the code - took me a while to realise the assembly is > > > "backwards" compared to the original Intel definition :-) - but I did not spot > > > anything obvious :-( > > > > > >> I guess it only ECB part ... > > > > Mystery solved, the skcipher subreq must be te last member in the struct. > > (Some comments in Adiantum code mentions it too, so I do not think it > > just hides the corruption after the struct. Seems like another magic requirement > > in crypto API :-) > > Oh, yes, this makes sense! I would have noticed this immediately if I > had looked carefully at the struct definition :) The reason is that > the skcipher_request struct is followed by a variable-length request > context. So when you want to nest requests, you need to make the > subrequest the last member and declare your request context size as: > size of your request context struct + size of the sub-algorithm's > request context. > > It is a bit confusing, but it is the only reasonable way to support > variably sized context and at the same time keep the whole request in > a single allocation. > Ah, ok, I did not know anything about that ... so there's really no way I could've done this correctly or to have found the problem myself really. Good that it's resolved now, though. I fixed a couple of other minor things already, is it OK if I roll this into an update to my original patch? Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
