Ticket #1263 (closed defect: fixed)

Opened 5 years ago

Last modified 2 years ago

HashString::cast_from functions

Reported by: croko Owned by: rakshasa
Priority: normal Component: libtorrent
Version: HEAD Severity: critical
Keywords: Cc:

Description

Hi, I am using svn HEAD versions of rtorrent & libtorrent on debian ARM and ran into following problem: I added a couple of torrents and let them download a while with the firewall blocking the access to the rtorrent listening ports. As soon as I turned on the port forwarding on the firewall rtorrent crashed with SIGBUS in function DownloadManager::find_main_obfuscated() (I don't know why the function was not called with the ports blocked by the firewall).

I fixed the error by modifying the start part of the function as below: DownloadMain?* DownloadManager::find_main_obfuscated(const char* hash) {

HashString? hs; hs.assign(hash); iterator itr = std::find_if(begin(), end(), rak::equal(hs,*HashString::cast_from(hash), ....

The DownloadManager::find_main() should be fixed in a similar manner.

The call to HashString::cast_from() was generating the error and actually all cast_from functions from HashString? are completely wrong. They are type casting a pure char string pointer to a class pointer. It's like having 2 unrelated classes A and B, a pointer pA pointing to class A and then the said pointer is type casted to class B and used to access member functions from class B. Of course such usage is compilable but will generate runtime errors since the member functions of class B do not exist in class A.

Except for the small bugs that are normal for a program under development I find your software very good and I am really happy with its performance.

Regards

Change History

comment:1 follow-up: ↓ 2 Changed 5 years ago by anonymous

This part of the code is used for encrypted connections. Presumably you never open outgoing encrypted connections so it didn't crash until you got incoming connections that are encrypted. You can disable allow_incoming in the encryption options to fix it that way as well.

Your analysis is wrong however, the crash isn't due to data types (C++ really doesn't work how you think it works), but rather because the string isn't aligned correctly and the memcmp implementation of your C library has bugs when accessing unaligned strings. Copying the string to a local variable forces it to be re-aligned but also wastes CPU cycles, so it is not a generally adequate solution.

Please state what version of the compiler and C library you were using to compile libtorrent, and make sure you've tried the latest versions of each too and that they still have this bug.

comment:2 in reply to: ↑ 1 Changed 5 years ago by anonymous

My bad, you are right about the C++ casting it should generate the right code but on the other hand I don't understand why "assign" which uses the std::memcpy doesn't crash even if it uses the same unaligned string. The compiler version I used is g++-3.4 and C library version is 2.3.6.

comment:3 Changed 5 years ago by anonymous

Presumably memcpy doesn't have the bug that memcmp has. It's not the same code, after all.

comment:4 Changed 2 years ago by anonymous

I confirm it's still an issue on arm (--enable-unaligned). It's a DHT code. With debugging enabled and optimizations disabled it looks a bit nicer:

{{ # libtorrent-0.12.7 $ dmesg ... [83385.468310] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001 [83385.684821] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001 [83385.876749] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001 [83386.044737] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001 [83386.115265] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001

$ gdb -p 15024 (gdb) break *0x403153c0 Breakpoint 1 at 0x403153c0: file dht_hash_map.h, line 74. (gdb) continue Breakpoint 1, 0x403153c0 in torrent::hashstring_hash::operator() (this=0x1ab4632, n=...) at dht_hash_map.h:74 74 { return *(size_t*)(n.data() + hashstring_hash_ofs); } (gdb) print hashstring_hash_ofs $1 = 8 (gdb) print n.data() $2 = 0xbecb83a6 "\321\330\a\204qq\aD\215\030xV\001\036S\334\a\221=$e1:q9:get_peers1:t8:100079641:y1:qeqe@\240\204J@\001"

Whee!

#0 0x403153c0 in torrent::hashstring_hash::operator() (this=0x1ab4632, n=...) at dht_hash_map.h:74 #1 0x40318ec0 in std::tr1::detail::_Hash_code_base<torrent::HashString, std::pair<torrent::HashString const, torrent::DhtTracker*>, std::_Select1st<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::equal_to<torrent::HashString>, torrent::hashstring_hash, std::tr1::detail::_Mod_range_hashing, std::tr1::detail::_Default_ranged_hash, false>::_M_hash_code (

this=0x1ab4630, k=...) at /usr/lib/gcc/armv5tel-softfloat-linux-gnueabi/4.4.5/include/g++-v4/tr1_impl/hashtable_policy.h:754

#2 0x403169d4 in std::tr1::_Hashtable<torrent::HashString, std::pair<torrent::HashString const, torrent::DhtTracker*>, std::allocator<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::_Select1st<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::equal_to<torrent::HashString>, torrent::hashstring_hash, std::tr1::detail::_Mod_range_hashing, std::tr1::detail::_Default_ranged_hash, std::tr1::detail::_Prime_rehash_policy, false, false, true>::find (this=0x1ab4630,

k=...) at /usr/lib/gcc/armv5tel-softfloat-linux-gnueabi/4.4.5/include/g++-v4/tr1_impl/hashtable:784

#3 0x40311bc8 in torrent::DhtRouter::get_tracker (this=0x1ab4450, hash=..., create=false) at dht_router.cc:159 #4 0x403203b0 in torrent::DhtServer::create_get_peers_response (this=0x1ab44a0, req=..., sa=0xbecb8d34, reply=...)

at dht_server.cc:321

#5 0x4031fe6c in torrent::DhtServer::process_query (this=0x1ab44a0, id=..., sa=0xbecb8d34, msg=...) at dht_server.cc:284 #6 0x40322ca8 in torrent::DhtServer::event_read (this=0x1ab44a0) at dht_server.cc:748 #7 0x402aa634 in torrent::PollEPoll::perform (this=0x19d3ec0) at poll_epoll.cc:170 #8 0x0011f1bc in core::PollManagerEPoll::poll (this=0x19d3c90, timeout=...) at poll_manager_epoll.cc:83 #9 0x00026aa0 in main (argc=1, argv=0xbecb90e4) at main.cc:843 }}

comment:5 Changed 2 years ago by anonymous

# libtorrent-0.12.7
$ dmesg
...
[83385.468310] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001
[83385.684821] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001
[83385.876749] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001
[83386.044737] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001
[83386.115265] Alignment trap: rtorrent (15024) PC=0x403153c0 Instr=0xe5933000 Address=0xbecb83ae FSR 0x001

$ gdb -p 15024
(gdb) break *0x403153c0
Breakpoint 1 at 0x403153c0: file dht_hash_map.h, line 74.
(gdb) continue
Breakpoint 1, 0x403153c0 in torrent::hashstring_hash::operator() (this=0x1ab4632, n=...) at dht_hash_map.h:74
74        { return *(size_t*)(n.data() + hashstring_hash_ofs); }
(gdb) print hashstring_hash_ofs
$1 = 8
(gdb) print n.data()
$2 = 0xbecb83a6 "\321\330\a\204qq\aD\215\030xV\001\036S\334\a\221=$e1:q9:get_peers1:t8:100079641:y1:qeqe@\240\204J@\001"

Whee!

#0  0x403153c0 in torrent::hashstring_hash::operator() (this=0x1ab4632, n=...) at dht_hash_map.h:74
#1  0x40318ec0 in std::tr1::__detail::_Hash_code_base<torrent::HashString, std::pair<torrent::HashString const, torrent::DhtTracker*>, std::_Select1st<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::equal_to<torrent::HashString>, torrent::hashstring_hash, std::tr1::__detail::_Mod_range_hashing, std::tr1::__detail::_Default_ranged_hash, false>::_M_hash_code (
    this=0x1ab4630, __k=...)
    at /usr/lib/gcc/armv5tel-softfloat-linux-gnueabi/4.4.5/include/g++-v4/tr1_impl/hashtable_policy.h:754
#2  0x403169d4 in std::tr1::_Hashtable<torrent::HashString, std::pair<torrent::HashString const, torrent::DhtTracker*>, std::allocator<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::_Select1st<std::pair<torrent::HashString const, torrent::DhtTracker*> >, std::equal_to<torrent::HashString>, torrent::hashstring_hash, std::tr1::__detail::_Mod_range_hashing, std::tr1::__detail::_Default_ranged_hash, std::tr1::__detail::_Prime_rehash_policy, false, false, true>::find (this=0x1ab4630, 
    __k=...) at /usr/lib/gcc/armv5tel-softfloat-linux-gnueabi/4.4.5/include/g++-v4/tr1_impl/hashtable:784
#3  0x40311bc8 in torrent::DhtRouter::get_tracker (this=0x1ab4450, hash=..., create=false) at dht_router.cc:159
#4  0x403203b0 in torrent::DhtServer::create_get_peers_response (this=0x1ab44a0, req=..., sa=0xbecb8d34, reply=...)
    at dht_server.cc:321
#5  0x4031fe6c in torrent::DhtServer::process_query (this=0x1ab44a0, id=..., sa=0xbecb8d34, msg=...) at dht_server.cc:284
#6  0x40322ca8 in torrent::DhtServer::event_read (this=0x1ab44a0) at dht_server.cc:748
#7  0x402aa634 in torrent::PollEPoll::perform (this=0x19d3ec0) at poll_epoll.cc:170
#8  0x0011f1bc in core::PollManagerEPoll::poll (this=0x19d3c90, timeout=...) at poll_manager_epoll.cc:83
#9  0x00026aa0 in main (argc=1, argv=0xbecb90e4) at main.cc:843

comment:6 Changed 2 years ago by anonymous

In other words n.data() isn't aligned enougs (addr % 4 = 2).

The other suspicious place is '*(size_t*)'. It should be *(uint32_t*), right? Or it does not come right from peer socket?

comment:7 Changed 2 years ago by anonymous

Reproducible on current SVN as well.

comment:8 Changed 2 years ago by rakshasa

  • Status changed from new to closed
  • Resolution set to fixed

Fixed the above issues with alignement in HEAD.

Note: See TracTickets for help on using tickets.