> -----Original Message----- > From: Milan Broz <gmazyland@xxxxxxxxx> > Sent: Wednesday, August 7, 2019 1:42 PM > To: Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>; Pascal van Leeuwen > <pascalvanl@xxxxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx > Cc: rsnel@xxxxxxxxxxxxxxx; herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx > Subject: Re: [PATCH] crypto: xts - Add support for Cipher Text Stealing > > On 07/08/2019 13:27, Pascal Van Leeuwen wrote: > >> On 07/08/2019 10:15, Pascal Van Leeuwen wrote: > >>> I went through the code a couple of times, but I cannot spot any mistakes in > >>> the lengths I'm using. Is it possible that your application is supplying a > >>> buffer that is just not large enough? > >> > >> 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 ... > > >> Seems something is rewritten in call > >> crypto_skcipher_encrypt(subreq); > >> > >> (after that call, I see rctx->rem_bytes set to 32, that does not make sense...) > >> Depending on what you have observed so far, it could also be corrupted earlier, i.e. during one of the skcipher_request* calls inside init_crypt? Notably, the rem_bytes variable is immediately behind the subreq struct inside rctx, but obviously these calls should not write beyond the end of that struct (as without those added variables, those writes would end up outside of the allocated rctx struct) > > Eh ... no, it should never become > 15 ... if it gets set to 32 somehow, > > then I can at least explain why that would result in a buffer overflow :-) > > Yes, that explains it. > > (And rewriting this value back does not help, I got different test vector output, but no > crash.) > Well, there's also an is_encrypt flag immediately behind rem_bytes that likely also gets corrupted? > Milan Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
