Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid memcpy in SharedSecret::new #303

Closed
TheBlueMatt opened this issue Jun 7, 2021 · 3 comments
Closed

Invalid memcpy in SharedSecret::new #303

TheBlueMatt opened this issue Jun 7, 2021 · 3 comments

Comments

@TheBlueMatt
Copy link
Member

Haven't seen this on x86, but it may be because the memcpy is being optimized away and hidden from valgrind, but on ppcel64 I see the following errors in valgrind. The various rust-lightning and C bits on the outside shouldn't be relevant, ultimately we call SharedSecret::new with two safely-constructed objects. Note that this is built against the HEAD of pull #279 though that shouldn't be related either.

==525==    at 0x487F4D4: memcpy (vg_replace_strmem.c:1034)
==525==    by 0x514D023: rustsecp256k1_v0_4_0_ge_neg (group_impl.h:84)
==525==    by 0x5151FBF: rustsecp256k1_v0_4_0_ecmult_const (ecmult_const_impl.h:242)
==525==    by 0x515D00F: rustsecp256k1_v0_4_0_ecdh (main_impl.h:53)
==525==    by 0x5091ECF: secp256k1::ecdh::SharedSecret::new (ecdh.rs:108)
==525==    by 0x5096C37: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::outbound_noise_act (peer_channel_encryptor.rs:192)
==525==    by 0x5041B9F: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::get_act_one (peer_channel_encryptor.rs:248)
==525==    by 0x4E9F69F: lightning::ln::peer_handler::PeerManager<Descriptor,CM,RM,L>::new_outbound_connection (peer_handler.rs:464)
==525==    by 0x4FCFD3B: PeerManager_new_outbound_connection (peer_handler.rs:751)
==525==    by 0x114E83: PeersConnection::PeersConnection(LDK::ChannelManager&, LDK::ChannelManager&, LDK::PeerManager&, LDK::PeerManager&) (demo.cpp:261)
==525==    by 0x10DDAF: main (demo.cpp:406)
==525== 
==525== Source and destination overlap in memcpy(0x1ffeffc808, 0x1ffeffc808, 88)
==525==    at 0x487F4D4: memcpy (vg_replace_strmem.c:1034)
==525==    by 0x514D023: rustsecp256k1_v0_4_0_ge_neg (group_impl.h:84)
==525==    by 0x5152007: rustsecp256k1_v0_4_0_ecmult_const (ecmult_const_impl.h:247)
==525==    by 0x515D00F: rustsecp256k1_v0_4_0_ecdh (main_impl.h:53)
==525==    by 0x5091ECF: secp256k1::ecdh::SharedSecret::new (ecdh.rs:108)
==525==    by 0x5096C37: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::outbound_noise_act (peer_channel_encryptor.rs:192)
==525==    by 0x5041B9F: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::get_act_one (peer_channel_encryptor.rs:248)
==525==    by 0x4E9F69F: lightning::ln::peer_handler::PeerManager<Descriptor,CM,RM,L>::new_outbound_connection (peer_handler.rs:464)
==525==    by 0x4FCFD3B: PeerManager_new_outbound_connection (peer_handler.rs:751)
==525==    by 0x114E83: PeersConnection::PeersConnection(LDK::ChannelManager&, LDK::ChannelManager&, LDK::PeerManager&, LDK::PeerManager&) (demo.cpp:261)
==525==    by 0x10DDAF: main (demo.cpp:406)
==525== 
==525== Source and destination overlap in memcpy(0x1ffeffc808, 0x1ffeffc808, 88)
==525==    at 0x487F4D4: memcpy (vg_replace_strmem.c:1034)
==525==    by 0x514F84F: rustsecp256k1_v0_4_0_ge_mul_lambda (group_impl.h:654)
==525==    by 0x515201B: rustsecp256k1_v0_4_0_ecmult_const (ecmult_const_impl.h:248)
==525==    by 0x515D00F: rustsecp256k1_v0_4_0_ecdh (main_impl.h:53)
==525==    by 0x5091ECF: secp256k1::ecdh::SharedSecret::new (ecdh.rs:108)
==525==    by 0x5096C37: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::outbound_noise_act (peer_channel_encryptor.rs:192)
==525==    by 0x5041B9F: lightning::ln::peer_channel_encryptor::PeerChannelEncryptor::get_act_one (peer_channel_encryptor.rs:248)
==525==    by 0x4E9F69F: lightning::ln::peer_handler::PeerManager<Descriptor,CM,RM,L>::new_outbound_connection (peer_handler.rs:464)
==525==    by 0x4FCFD3B: PeerManager_new_outbound_connection (peer_handler.rs:751)
==525==    by 0x114E83: PeersConnection::PeersConnection(LDK::ChannelManager&, LDK::ChannelManager&, LDK::PeerManager&, LDK::PeerManager&) (demo.cpp:261)
==525==    by 0x10DDAF: main (demo.cpp:406)
@TheBlueMatt
Copy link
Member Author

This appears to be a gcc mis-compilation moved to bitcoin-core/secp256k1#949.

@tcharding
Copy link
Member

If this is to be fixed in bitcoin core can we close this issue @TheBlueMatt?

@apoelstra
Copy link
Member

Yep, let's close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants