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.
-~----------~----~----~----~------~----~------~--~---
Index: zdeflate.cpp
===================================================================
--- zdeflate.cpp (revision 772)
+++ zdeflate.cpp (working copy)
@@ -214,6 +214,7 @@
Deflator::Deflator(BufferedTransformation *attachment, int deflateLevel, int log2WindowSize, bool detectUncompressible)
: LowFirstBitWriter(attachment)
+ , m_deflateLevel(-1)
{
InitializeStaticEncoders();
IsolatedInitialize(MakeParameters("DeflateLevel", deflateLevel)("Log2WindowSize", log2WindowSize)("DetectUncompressible", detectUncompressible));
