Sorry for the late response. I've confirmed these issues, but want to fix
them a bit differently. Chris, please try the attached patch and let me know
if it fixes the valgrind problems.
----- Original Message -----
From: "Chris Morgan" <[EMAIL PROTECTED]>
To: "Crypto++ Users" <[EMAIL PROTECTED]>; "Wei Dai"
<[EMAIL PROTECTED]>
Sent: Tuesday, August 07, 2007 6:26 PM
Subject: Re: [PATCH] - fix some issues identified by valgrind
>
> Any news on these patches?
>
> Chris
>
>
>
> On 8/5/07, Chris Morgan <[EMAIL PROTECTED]> wrote:
>> Detected these issues while running the cryptest.exe validation under
>> valgrind.
>>
>> The zdeflate.cpp change was because SetDeflateLevel() was using
>> m_deflateLevel before m_deflateLevel was initialized. I'm not sure
>> this is the way you would want to fix the issue since there are a
>> couple of different way to correct this.
>>
>>
>> words.h code was resulting in an issue because we were calling
>> memcpy() with the same source and destination, something the man page
>> for memcpy() says not to do. Here is the output from valgrind. It
>> seems as if crypto++ could optimize this situation away higher up in
>> the architecture but I wasn't sure.
>>
>> ==13599== Source and destination overlap in memcpy(0x4068098, 0x4068098,
>> 16)
>> ==13599== at 0x4C237F3: memcpy (mc_replace_strmem.c:116)
>> ==13599== by 0x535756: CryptoPP::PositiveAdd(CryptoPP::Integer&,
>> CryptoPP::Integer const&, CryptoPP::Integer const&) (words.h:24)
>> ==13599== by 0x535A64:
>> CryptoPP::Integer::operator+=(CryptoPP::Integer const&)
>> (integer.cpp:3559)
>> ==13599== by 0x535FA2: CryptoPP::Integer
>> CryptoPP::StringToInteger<char>(char const*) (integer.cpp:3014)
>> ==13599== by 0x53611E: CryptoPP::Integer::Integer(char const*)
>> (integer.cpp:3027)
>> ==13599== by 0x472054: ValidateBBS() (validat2.cpp:55)
>> ==13599== by 0x461DFC: ValidateAll(bool) (validat1.cpp:97)
>> ==13599== by 0x44A2A6: Validate(int, bool, char const*) (test.cpp:781)
>> ==13599== by 0x451C57: main (test.cpp:298)
>> passed 49ea2cfdb01064a0bbb92af101dac18a94f7b7ce
>> passed 2af101dac18a94f7b7ce
>>
>>
>> Similar change in filters.cpp as in words.h, again, here is the
>> valgrind output in case it helps figure out why the higher level code
>> is calling the lower level code with the same source and destination.
>>
>> passed cryptosystem key validation
>> ==13097== Source and destination overlap in memcpy(0x5ED05A0, 0x5ED05A0,
>> 20)
>> ==13097== at 0x4C237F3: memcpy (mc_replace_strmem.c:116)
>> ==13097== by 0x501BF7: CryptoPP::ArraySink::Put2(unsigned char
>> const*, unsigned long, int, bool) (filters.cpp:512)
>> ==13097== by 0x501368: CryptoPP::Filter::Output(int, unsigned char
>> const*, unsigned long, int, bool, std::string const&)
>> (filters.cpp:115)
>> ==13097== by 0x503073: CryptoPP::HashFilter::Put2(unsigned char
>> const*, unsigned long, int, bool) (filters.cpp:730)
>> ==13097== by 0x55B7B9:
>> CryptoPP::P1363_MGF1KDF2_Common(CryptoPP::HashTransformation&,
>> unsigned char*, unsigned long, unsigned char const*, unsigned long,
>> unsigned char const*, unsigned long, bool, unsigned) (cryptlib.h:763)
>> ==13097== by 0x4391AE:
>> CryptoPP::DL_KeyDerivationAlgorithm_P1363<CryptoPP::Integer, true,
>> CryptoPP::P1363_KDF2<CryptoPP::SHA1>
>> >::Derive(CryptoPP::DL_GroupParameters<CryptoPP::Integer> const&,
>> unsigned char*, unsigned long, CryptoPP::Integer const&,
>> CryptoPP::Integer const&, CryptoPP::NameValuePairs const&) const
>> (pubkey.h:506)
>> ==13097== by 0x4325BF:
>> CryptoPP::DL_EncryptorBase<CryptoPP::Integer>::Encrypt(CryptoPP::RandomNumberGenerator&,
>> unsigned char const*, unsigned long, unsigned char*,
>> CryptoPP::NameValuePairs const&) const (pubkey.h:1237)
>> ==13097== by 0x471324:
>> CryptoSystemValidate(CryptoPP::PK_Decryptor&, CryptoPP::PK_Encryptor&,
>> bool) (validat2.cpp:169)
>> ==13097== by 0x474794: ValidateDLIES() (validat2.cpp:399)
>> ==13097== by 0x461E40: ValidateAll(bool) (validat1.cpp:102)
>> ==13097== by 0x44A2A6: Validate(int, bool, char const*) (test.cpp:781)
>> ==13097== by 0x451C57: main (test.cpp:298)
>> passed encryption and decryption
>>
>>
>> With these three changes the valgrind output here looks good. No more
>> errors and no leaks, btw I'm using the --leak-check=full option as
>> well.
>>
>> Chris
>>
>>
>
> >
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the "Crypto++ Users"
Google Group.
To unsubscribe, send an email to [EMAIL PROTECTED]
More information about Crypto++ and this group is available at
http://www.cryptopp.com.
-~----------~----~----~----~------~----~------~--~---
Index: filters.cpp
===================================================================
--- filters.cpp (revision 379)
+++ filters.cpp (working copy)
@@ -509,7 +509,8 @@
size_t ArraySink::Put2(const byte *begin, size_t length, int messageEnd, bool
blocking)
{
- memcpy(m_buf+m_total, begin, STDMIN(length, SaturatingSubtract(m_size,
m_total)));
+ if (m_buf+m_total != begin)
+ memcpy(m_buf+m_total, begin, STDMIN(length,
SaturatingSubtract(m_size, m_total)));
m_total += length;
return 0;
}
Index: words.h
===================================================================
--- words.h (revision 379)
+++ words.h (working copy)
@@ -20,7 +20,8 @@
inline void CopyWords(word *r, const word *a, size_t n)
{
- memcpy(r, a, n*WORD_SIZE);
+ if (r != a)
+ memcpy(r, a, n*WORD_SIZE);
}
inline void XorWords(word *r, const word *a, const word *b, size_t n)
Index: zdeflate.cpp
===================================================================
--- zdeflate.cpp (revision 379)
+++ zdeflate.cpp (working copy)
@@ -221,6 +221,7 @@
Deflator::Deflator(const NameValuePairs ¶meters, BufferedTransformation
*attachment)
: LowFirstBitWriter(attachment)
+ , m_deflateLevel(-1)
{
InitializeStaticEncoders();
IsolatedInitialize(parameters);
