Hi Wei. Any update on this patch and the other valgrind issue? Do you need anything else from me at this point?
Chris On Jan 19, 2008 2:17 PM, Chris Morgan <[EMAIL PROTECTED]> wrote: > Hey Wei. Thanks for fixing those issues. > > I've upgraded from either 5.5.0 or 5.5.1 to 5.5.2 and valgrind finds a > couple more issues. > > ... > Testing SymmetricCipher algorithm Salsa20. > ......==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D125: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (char_traits.h:254) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A6AC: ValidateSalsa() (validat1.cpp:1339) > ==21004== by 0x461DF4: ValidateAll(bool) (validat1.cpp:93) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > ==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D144: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (datatest.cpp:321) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A6AC: ValidateSalsa() (validat1.cpp:1339) > ==21004== by 0x461DF4: ValidateAll(bool) (validat1.cpp:93) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > ==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D41B: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (char_traits.h:254) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A6AC: ValidateSalsa() (validat1.cpp:1339) > ==21004== by 0x461DF4: ValidateAll(bool) (validat1.cpp:93) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > . > Tests complete. Total tests = 7. Failed tests = 0. > > Sosemanuk validation suite running... > > Testing SymmetricCipher algorithm Sosemanuk. > ==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D125: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (char_traits.h:254) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A67C: ValidateSosemanuk() (validat1.cpp:1345) > ==21004== by 0x461E01: ValidateAll(bool) (validat1.cpp:94) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > ==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D41B: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (char_traits.h:254) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A67C: ValidateSosemanuk() (validat1.cpp:1345) > ==21004== by 0x461E01: ValidateAll(bool) (validat1.cpp:94) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > .==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D144: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (datatest.cpp:321) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A67C: ValidateSosemanuk() (validat1.cpp:1345) > ==21004== by 0x461E01: ValidateAll(bool) (validat1.cpp:94) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > ==21004== > ==21004== Conditional jump or move depends on uninitialised value(s) > ==21004== at 0x48D41D: TestSymmetricCipher(std::map<std::string, > std::string, std::less<std::string>, > std::allocator<std::pair<std::string const, std::string> > >&) > (char_traits.h:254) > ==21004== by 0x48F832: TestDataFile(std::string const&, unsigned&, > unsigned&) (datatest.cpp:517) > ==21004== by 0x48FCA8: RunTestDataFile(char const*) (datatest.cpp:559) > ==21004== by 0x45A67C: ValidateSosemanuk() (validat1.cpp:1345) > ==21004== by 0x461E01: ValidateAll(bool) (validat1.cpp:94) > ==21004== by 0x44A064: Validate(int, bool, char const*) (test.cpp:781) > ==21004== by 0x45190C: main (test.cpp:298) > . > Tests complete. Total tests = 2. Failed tests = 0. > > VMAC validation suite running... > ... > > > This appears to indicate that datatest.cpp:321 has a comparison with > an uninitialized value. I can't see where encrypted, ciphertext, > xorDigest or ciphertextXorDigest are initialized but it appears that > one of those, or test, is not initialized. I was having a hard time > tracking this one down because I don't understand the code well and > hoping you or someone else familiar with the code could quickly spot > it. > > > I've attached another patch for zdeflate.cpp. Apparently another > version of the constructor is being called with 5.5.2 which brought > out the same initialization issue with m_deflateLevel, but with > another variant of the constructor. > > Chris > > > > > > On Aug 11, 2007 8:27 PM, Wei Dai <[EMAIL PROTECTED]> wrote: > > 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. -~----------~----~----~----~------~----~------~--~---
