On Tue 16 Oct 2018 12:09:14 PM CEST, Daniel P. Berrangé wrote:
> @@ -110,20 +111,34 @@ void xts_decrypt(const void *datactx,
> /* encrypt the iv */
> encfunc(tweakctx, XTS_BLOCK_SIZE, T.b, iv);
>
> - for (i = 0; i < lim; i++) {
> - xts_tweak_encdec(datactx, decfunc, src, dst, T.b);
> -
> - src += XTS_BLOCK_SIZE;
> - dst += XTS_BLOCK_SIZE;
> + if (QEMU_PTR_IS_ALIGNED(src, sizeof(uint64_t)) &&
> + QEMU_PTR_IS_ALIGNED(dst, sizeof(uint64_t))) {
> + xts_uint128 *S = (xts_uint128 *)src;
> + xts_uint128 *D = (xts_uint128 *)dst;
> + for (i = 0; i < lim; i++, S++, D++) {
> + xts_tweak_encdec(datactx, decfunc, S, D, &T);
> + }
> + } else {
> + xts_uint128 S, D;
> +
> + for (i = 0; i < lim; i++) {
> + memcpy(&S, src, XTS_BLOCK_SIZE);
> + xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
> + memcpy(dst, &D, XTS_BLOCK_SIZE);
> + src += XTS_BLOCK_SIZE;
> + dst += XTS_BLOCK_SIZE;
> + }
The patch looks good to me, but a couple of comments:
- As far as I can see xts_tweak_encdec() works the same regardless of
whether src and dst point to the same address or not. As a matter of
fact both qcrypto_block_decrypt() and qcrypto_block_encrypt() do the
decryption and encryption in place, and as you can see the
qcrypto_cipher_*crypt() calls in crypto/block.c pass the same buffer
as input and output.
So instead of having S and D you should be fine with just one of them.
- I think this is just a matter of style preference, but in the first
for loop you can remove the comma operator (i++, S++, D++) and use
S[i] and D[I] instead in the line after that. I'm fine if you prefer
the current style, though.
Berto
