valgrind: Conditional jump or move depends on uninitialised value
malytomas opened this issue · 7 comments
malytomas commented
I get an error detected by valgrind when running my tests.
==15235== Thread 6 steam sockets:
==15235== Conditional jump or move depends on uninitialised value(s)
==15235== at 0x5E4B5E4: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==15235== by 0x5E4B8C1: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==15235== by 0x5D4C1E4: EVP_DecryptFinal_ex (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==15235== by 0x4E237C4: AES_GCM_DecryptContext::Decrypt(void const*, unsigned long, void const*, void*, unsigned int*, void const*, unsigned long) (crypto_symmetric_opensslevp.cpp:246)
==15235== by 0x4E4EC81: SteamNetworkingSocketsLib::CSteamNetworkConnectionBase::DecryptDataChunk(unsigned short, int, void const*, int, SteamNetworkingSocketsLib::RecvPacketContext_t&) (steamnetworkingsockets_connections.cpp:2177)
==15235== by 0x4E8BD8A: SteamNetworkingSocketsLib::CConnectionTransportUDPBase::Received_Data(unsigned char const*, int, long long) (steamnetworkingsockets_udp.cpp:747)
==15235== by 0x4E8DD17: SteamNetworkingSocketsLib::CConnectionTransportUDP::PacketReceived(SteamNetworkingSocketsLib::RecvPktInfo_t const&, SteamNetworkingSocketsLib::CConnectionTransportUDP*) (steamnetworkingsockets_udp.cpp:1377)
==15235== by 0x4E94E06: operator() (steamnetworkingsockets_lowlevel.h:83)
==15235== by 0x4E94E06: DrainSocket (steamnetworkingsockets_lowlevel.cpp:1989)
==15235== by 0x4E94E06: SteamNetworkingSocketsLib::PollRawUDPSockets(int, bool) (steamnetworkingsockets_lowlevel.cpp:2140)
==15235== by 0x4E95555: SteamNetworkingSocketsLib::SteamNetworkingSockets_InternalPoll(int, bool) (steamnetworkingsockets_lowlevel.cpp:2736)
==15235== by 0x4E957E2: SteamNetworkingSocketsLib::SteamNetworkingThreadProc() (steamnetworkingsockets_lowlevel.cpp:2854)
==15235== by 0x56DD2B2: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==15235== by 0x59C8B42: start_thread (pthread_create.c:442)
==15235==
I am compiling the code with g++ version 12 running on ubuntu 22 with valgrind tool memcheck.
Please let me know if you need any more information.
zpostfacto commented
Is this a bug in openssl?
malytomas commented
Is this a bug in openssl?
I honestly hope not. ;)
I consider it more likely to be in the game sockets than in the openssl - purely based on the exposure and "criticality" of openssl.
If I were to guess, I would look for uninitialized padding bytes (or unused member variable) in some structure being passed to openssl and treated as byte buffer there. But this is really just a wild guess.
Can you replicate it with valgrind in your tests?
zpostfacto commented
Can you run your test with 36fc6f8 ?
Also, if you have a test setup to compile and run tests with valgrind, I would like to add that to my test suite. I would pull your change if you know the relevant cmake. Everything in cmake is so slow for me, this is not likely to be something I would do on my own.
zpostfacto commented
Oops, one more. 7f98753
malytomas commented
test_crypto: (click to expand)
+ valgrind --track-origins=yes build-cmake/bin/test_crypto
==25303== Memcheck, a memory error detector
==25303== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==25303== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==25303== Command: build-cmake/bin/test_crypto
==25303==
==25303== Conditional jump or move depends on uninitialised value(s)
==25303== at 0x4DB95E4: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25303== by 0x4DB98C1: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25303== by 0x4CBA1E4: EVP_DecryptFinal_ex (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25303== by 0x12E99A: AES_GCM_DecryptContext::Decrypt(void const*, unsigned long, void const*, void*, unsigned int*, void const*, unsigned long) (crypto_symmetric_opensslevp.cpp:264)
==25303== by 0x11A065: TestSymmetricAuthCrypto_EncryptTestVectorFile(char const*) (test_crypto.cpp:278)
==25303== by 0x116DD6: TestSymmetricAuthCryptoVectors (test_crypto.cpp:312)
==25303== by 0x116DD6: main (test_crypto.cpp:755)
==25303==
This one unfortunately does not report the origin of the uninitialized variable.
test_connection suite-quick (CUtlMemoryBase::Grow): (click to expand)
==25312== Thread 2:
==25312== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==25312== at 0x540DC16: sendto (sendto.c:27)
==25312== by 0x140CEB: BReallySendRawPacket (steamnetworkingsockets_lowlevel.cpp:1023)
==25312== by 0x140CEB: SteamNetworkingSocketsLib::CPacketLaggerSend::ProcessPacket(SteamNetworkingSocketsLib::CPacketLagger::LaggedPacket const&, long long) (steamnetworkingsockets_lowlevel.cpp:1288)
==25312== by 0x13F646: SteamNetworkingSocketsLib::CPacketLagger::Think(long long) (steamnetworkingsockets_lowlevel.cpp:1223)
==25312== by 0x1A5DFE: SteamNetworkingSocketsLib::IThinker::Thinker_ProcessThinkers() (steamnetworkingsockets_thinker.cpp:229)
==25312== by 0x145AA4: SteamNetworkingSocketsLib::SteamNetworkingSockets_InternalPoll(int, bool) (steamnetworkingsockets_lowlevel.cpp:2752)
==25312== by 0x145BF2: SteamNetworkingSocketsLib::SteamNetworkingThreadProc() (steamnetworkingsockets_lowlevel.cpp:2854)
==25312== by 0x50522B2: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.32)
==25312== by 0x537AB42: start_thread (pthread_create.c:442)
==25312== by 0x540BBB3: clone (clone.S:100)
==25312== Address 0x577fa33 is 83 bytes inside a block of size 21,632 alloc'd
==25312== at 0x484DCD3: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==25312== by 0x19D3EC: SteamNetworkingSocketsTier1::CUtlMemoryBase::Grow(int) (utlmemory.cpp:266)
==25312== by 0x14446C: AllocInternal (utllinkedlist.h:397)
==25312== by 0x14446C: CUtlLinkedList<SteamNetworkingSocketsLib::CPacketLagger::LaggedPacket, int>::InsertAfter(int) (utllinkedlist.h:470)
==25312== by 0x1445B3: SteamNetworkingSocketsLib::CPacketLagger::LagPacket(SteamNetworkingSocketsLib::CRawUDPSocketImpl*, CIPAndPort const&, int, int, iovec const*) (steamnetworkingsockets_lowlevel.cpp:1175)
==25312== by 0x144AB7: SteamNetworkingSocketsLib::CRawUDPSocketImpl::BSendRawPacketGather(int, iovec const*, CIPAndPort const&) const (steamnetworkingsockets_lowlevel.cpp:1435)
==25312== by 0x16B4B9: SteamNetworkingSocketsLib::CConnectionTransportUDPBase::SendEncryptedDataChunk(void const*, int, SteamNetworkingSocketsLib::SendPacketContext_t&) (steamnetworkingsockets_udp.cpp:570)
==25312== by 0x160CC6: SteamNetworkingSocketsLib::CSteamNetworkConnectionBase::SNP_SendPacket(SteamNetworkingSocketsLib::CConnectionTransport*, SteamNetworkingSocketsLib::SendPacketContext_t&) (steamnetworkingsockets_snp.cpp:1882)
==25312== by 0x16A872: SteamNetworkingSocketsLib::CConnectionTransportUDPBase::SendDataPacket(long long) (steamnetworkingsockets_udp.cpp:517)
==25312== by 0x159A54: SteamNetworkingSocketsLib::CSteamNetworkConnectionBase::SNP_ThinkSendState(long long) (steamnetworkingsockets_snp.cpp:4228)
==25312== by 0x13CA4D: SteamNetworkingSocketsLib::CSteamNetworkConnectionBase::Think(long long) (steamnetworkingsockets_connections.cpp:3485)
==25312== by 0x1A5DFE: SteamNetworkingSocketsLib::IThinker::Thinker_ProcessThinkers() (steamnetworkingsockets_thinker.cpp:229)
==25312== by 0x145AA4: SteamNetworkingSocketsLib::SteamNetworkingSockets_InternalPoll(int, bool) (steamnetworkingsockets_lowlevel.cpp:2752)
==25312==
test_connection suite-quick (libcrypto): (click to expand)
==25312== Conditional jump or move depends on uninitialised value(s)
==25312== at 0x4DB95E4: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25312== by 0x4DB98C1: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25312== by 0x4CBA1E4: EVP_DecryptFinal_ex (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25312== by 0x190A4A: AES_GCM_DecryptContext::Decrypt(void const*, unsigned long, void const*, void*, unsigned int*, void const*, unsigned long) (crypto_symmetric_opensslevp.cpp:264)
==25312== by 0x134B81: SteamNetworkingSocketsLib::CSteamNetworkConnectionBase::DecryptDataChunk(unsigned short, int, void const*, int, SteamNetworkingSocketsLib::RecvPacketContext_t&) (steamnetworkingsockets_connections.cpp:2177)
==25312== by 0x16AF23: SteamNetworkingSocketsLib::CConnectionTransportUDPBase::Received_Data(unsigned char const*, int, long long) (steamnetworkingsockets_udp.cpp:747)
==25312== by 0x16C810: SteamNetworkingSocketsLib::CConnectionTransportUDP::PacketReceived(SteamNetworkingSocketsLib::RecvPktInfo_t const&, SteamNetworkingSocketsLib::CConnectionTransportUDP*) (steamnetworkingsockets_udp.cpp:1377)
==25312== by 0x145288: operator() (steamnetworkingsockets_lowlevel.h:83)
==25312== by 0x145288: DrainSocket (steamnetworkingsockets_lowlevel.cpp:1989)
==25312== by 0x145288: SteamNetworkingSocketsLib::PollRawUDPSockets(int, bool) (steamnetworkingsockets_lowlevel.cpp:2140)
==25312== by 0x1459F1: SteamNetworkingSocketsLib::SteamNetworkingSockets_InternalPoll(int, bool) (steamnetworkingsockets_lowlevel.cpp:2736)
==25312== by 0x145BF2: SteamNetworkingSocketsLib::SteamNetworkingThreadProc() (steamnetworkingsockets_lowlevel.cpp:2854)
==25312== by 0x50522B2: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.32)
==25312== by 0x537AB42: start_thread (pthread_create.c:442)
==25312== Uninitialised value was created by a stack allocation
==25312== at 0x4E31063: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==25312==
This one shows the variable is inside libcrypto, which is indeed worrying.
zpostfacto commented
I'll take a look at the BReallySendRawPacket one.
The ones inside SSL with AES_GCM_DecryptContext::Decrypt: Do you agree that if any of those arguments were uninitialized, with my changes they should have fired in my code? (Just want a sanity check that my changes should have detected any bug on my side.)
zpostfacto commented
I'm going to close this. If it can be narrowed down to something that I can reproduce, I'd be interested in investigating.