diff --git a/strciphr.cpp b/strciphr.cpp index 7a15dd2f..d7777ed6 100644 --- a/strciphr.cpp +++ b/strciphr.cpp @@ -59,8 +59,8 @@ void AdditiveCipherTemplate::GenerateBlock(byte *outString, size_t length) size_t bufferByteSize = RoundUpToMultipleOf(length, bytesPerIteration); size_t bufferIterations = bufferByteSize / bytesPerIteration; - policy.WriteKeystream(KeystreamBufferEnd()-bufferByteSize, bufferIterations); - memcpy(outString, KeystreamBufferEnd()-bufferByteSize, length); + policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bufferByteSize), bufferIterations); + memcpy(outString, PtrSub(KeystreamBufferEnd(), bufferByteSize), length); m_leftOver = bufferByteSize - length; } } @@ -71,7 +71,7 @@ void AdditiveCipherTemplate::ProcessData(byte *outString, const byte *inStrin if (m_leftOver > 0) { const size_t len = STDMIN(m_leftOver, length); - xorbuf(outString, inString, KeystreamBufferEnd()-m_leftOver, len); + xorbuf(outString, inString, PtrSub(KeystreamBufferEnd(), m_leftOver), len); length -= len; m_leftOver -= len; inString = PtrAdd(inString, len); @@ -115,8 +115,8 @@ void AdditiveCipherTemplate::ProcessData(byte *outString, const byte *inStrin bufferByteSize = RoundUpToMultipleOf(length, bytesPerIteration); bufferIterations = bufferByteSize / bytesPerIteration; - policy.WriteKeystream(KeystreamBufferEnd()-bufferByteSize, bufferIterations); - xorbuf(outString, inString, KeystreamBufferEnd()-bufferByteSize, length); + policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bufferByteSize), bufferIterations); + xorbuf(outString, inString, PtrSub(KeystreamBufferEnd(), bufferByteSize), length); m_leftOver = bufferByteSize - length; } } @@ -141,7 +141,7 @@ void AdditiveCipherTemplate::Seek(lword position) if (position > 0) { - policy.WriteKeystream(KeystreamBufferEnd()-bytesPerIteration, 1); + policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bytesPerIteration), 1); m_leftOver = bytesPerIteration - (unsigned int)position; } else @@ -195,16 +195,6 @@ void CFB_CipherTemplate::ProcessData(byte *outString, const byte *inString if (!length) {return;} } - // TODO: Figure out what is happening on ARM A-32. x86, Aarch64 and PowerPC are OK. - // The issue surfaced for CFB mode when we cut-in Cryptogams AES ARMv7 asm. - // Using 'outString' for both input and output leads to incorrect results. - // - // Benchmarking on Cortex-A7 and Cortex-A9 indicates removing the block - // below does not have a material effect on performance. - // - // Also see https://github.com/weidai11/cryptopp/issues/683. - // -#if !defined(__arm__) if (policy.CanIterate() && length >= bytesPerIteration && IsAlignedOn(outString, alignment)) { const CipherDir cipherDir = GetCipherDir(*this); @@ -212,32 +202,6 @@ void CFB_CipherTemplate::ProcessData(byte *outString, const byte *inString policy.Iterate(outString, inString, cipherDir, length / bytesPerIteration); else { - // GCC and Clang does not like this on ARM. The incorrect result is a string - // of 0's instead of ciphertext (or plaintext if decrypting). The 0's trace - // back to the allocation for the std::string in datatest.cpp. Elements in the - // string are initialized to their default value, which is 0. - // - // It almost feels as if the compiler does not see the string is transformed - // in-place so it short-circuits the transform. However, if we use a stand-alone - // reproducer with the same data then the issue is _not_ present. - // - // When working on this issue we introduced PtrAdd and PtrSub to ensure we were - // not running afoul of pointer arithmetic rules of the language. Namely we need - // to use ptrdiff_t when subtracting pointers. We believe the relevant code paths - // are clean. - // - // There are two remaining open questions. The first is aliasing rules. Char-types - // are not bound by aliasing rules so we are OK. The second is array const-ness. - // The arrays are created in datatest.cpp and they are non-const. Since the original - // objects are non-const we are OK casting const-ness away as buffers are twiddled. - // - // One workaround is a distinct and aligned temporary buffer. It [mostly] works - // as expected but requires an extra allocation (casts not shown): - // - // std::string temp(length); - // std::memcpy(&temp[0], inString, length); - // policy.Iterate(outString, &temp[0], cipherDir, length / bytesPerIteration); - // memcpy(outString, inString, length); policy.Iterate(outString, outString, cipherDir, length / bytesPerIteration); } @@ -245,7 +209,6 @@ void CFB_CipherTemplate::ProcessData(byte *outString, const byte *inString outString = PtrAdd(outString, length - length % bytesPerIteration); length %= bytesPerIteration; } -#endif while (length >= bytesPerIteration) { diff --git a/strciphr.h b/strciphr.h index 9198b55a..b726d0a7 100644 --- a/strciphr.h +++ b/strciphr.h @@ -376,7 +376,7 @@ protected: unsigned int GetBufferByteSize(const PolicyInterface &policy) const {return policy.GetBytesPerIteration() * policy.GetIterationsToBuffer();} inline byte * KeystreamBufferBegin() {return this->m_buffer.data();} - inline byte * KeystreamBufferEnd() {return (this->m_buffer.data() + this->m_buffer.size());} + inline byte * KeystreamBufferEnd() {return (PtrAdd(this->m_buffer.data(), this->m_buffer.size()));} SecByteBlock m_buffer; size_t m_leftOver;