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

ringdht: Refactoring the ringaccount class #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hanou2691
Copy link

ringdht: Refactoring the ringaccount class. This creates a new class called contactsmanager which manages the accounts contacts.

This also transforms the contact struct into a new class called contact. This patch splits the responsabilty of managing the contacts list from the ringaccount class and gives it to a new class. Therefore, the ringaccount will call the contactmanager methods. This is introduces to improve the code quality. Due to this solution, the ringaccount class won't have too many responsabilities

GerritRingMirror pushed a commit that referenced this pull request Jun 9, 2021
`memcpy()` has the `__nonnull__` and ASAN doesn't like it even tho the length
of the buffer is 0.  Thus, using a dummy buffer on the stack.

--------------------------------------------------------------------------------
#0 0x55555a0a1b8a in /usr/include/msgpack/v1/sbuffer.hpp:74
#1 0x55555a1dcfd3 in /usr/include/msgpack/v1/pack.hpp:623
#3 0x55555a11eab2 in /usr/include/msgpack/v1/pack.hpp:1311
#4 0x55555a35c1c5 in /ring-project/daemon/src/jamidht/multiplexed_socket.cpp:676
#5 0x55555a363879 in /ring-project/daemon/src/jamidht/multiplexed_socket.cpp:945
#6 0x55555a35554e in /ring-project/daemon/src/jamidht/multiplexed_socket.cpp:459
#7 0x55555a34e0c0 in /ring-project/daemon/src/jamidht/multiplexed_socket.cpp:247
#8 0x55555a37298f in /ring-project/daemon/src/jamidht/multiplexed_socket.cpp:75
(...)
--------------------------------------------------------------------------------

Change-Id: Ibc8c8d808c233da1649f556466b24d68decf85e8
GerritRingMirror pushed a commit that referenced this pull request Jun 11, 2021
Jamiaccount add a listener while the state is been changed.  This can result in
reallocation of the underlying vector while it's been iterated, resulting in a
read after free.

--------------------------------------------------------------------------------
==930034==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000991900
READ of size 8 at 0x603000991900 thread T1
#0 0x55555a8a6dcb in /ring-project/daemon/src/call.cpp:94
#1 0x55555a8c8483 in /usr/include/c++/11.1.0/bits/invoke.h:61
#2 0x55555a8c654a in /usr/include/c++/11.1.0/bits/invoke.h:111
#3 0x55555a8c4c4e in /usr/include/c++/11.1.0/bits/std_function.h:291
#4 0x55555a8d5102 in /usr/include/c++/11.1.0/bits/std_function.h:560
#5 0x55555a8af158 in /ring-project/daemon/src/call.cpp:270
#6 0x55555a8aff7a in /ring-project/daemon/src/call.cpp:296
#7 0x55555a8b987d in /ring-project/daemon/src/call.cpp:575
#8 0x55555a8b5067 in /ring-project/daemon/src/call.cpp:482
#9 0x55555a8c225b in /ring-project/daemon/src/manager.h:1047
#10 0x55555a8ca928 in /usr/include/c++/11.1.0/bits/invoke.h:61
#11 0x55555a8c88d8 in /usr/include/c++/11.1.0/bits/invoke.h:111
#12 0x55555a8c6878 in /usr/include/c++/11.1.0/bits/std_function.h:291
#13 0x555559cff4a8 in /usr/include/c++/11.1.0/bits/std_function.h:560
#14 0x55555aaae8a1 in /ring-project/daemon/src/scheduled_executor.cpp:137
#15 0x55555aaaaf8f in /ring-project/daemon/src/scheduled_executor.cpp:32
#16 0x55555aab4a2f in /usr/include/c++/11.1.0/bits/invoke.h:61
#17 0x55555aab48ea in /usr/include/c++/11.1.0/bits/invoke.h:96
#18 0x55555aab47bf in /usr/include/c++/11.1.0/bits/std_thread.h:253
#19 0x55555aab46f5 in /usr/include/c++/11.1.0/bits/std_thread.h:260
#20 0x55555aab46ad in /usr/include/c++/11.1.0/bits/std_thread.h:211
#21 0x7ffff45583c3 in /build/gcc/src/gcc/libstdc++-v3/src/c++11/thread.cc:82
#22 0x7ffff649f258 in /usr/lib/libpthread.so.0+0x9258
#23 0x7ffff38e45e2 in /usr/lib/libc.so.6+0xfe5e2

0x603000991900 is located 0 bytes inside of 32-byte region [0x603000991900,0x603000991920)
freed by thread T0 here:
#0 0x7ffff769fd69 in /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172
#1 0x55555a1e3dc3 in /usr/include/c++/11.1.0/ext/new_allocator.h:139
#2 0x55555a18f942 in /usr/include/c++/11.1.0/bits/alloc_traits.h:492
#3 0x55555a12a9c1 in /usr/include/c++/11.1.0/bits/stl_vector.h:354
#4 0x55555a12b390 in /usr/include/c++/11.1.0/bits/vector.tcc:500
#5 0x55555a0e1a7c in /usr/include/c++/11.1.0/bits/vector.tcc:121
#6 0x55555a0b8c40 in /ring-project/daemon/src/call.h:286
#7 0x555559f43b69 in /usr/include/c++/11.1.0/bits/jamiaccount.cpp:675
#8 0x555559f3bf91 in /usr/include/c++/11.1.0/bits/jamiaccount.cpp:483
#9 0x555559f39cb7 in /usr/include/c++/11.1.0/bits/jamiaccount.cpp:449
#10 0x55555a838f0e in /ring-project/daemon/src/manager.cpp:3350
#11 0x55555a7f7aef in /ring-project/daemon/src/manager.cpp:1015
#12 0x555559d3c828 in /usr/include/c++/11.1.0/callmanager.cpp:67
#13 0x555559c70b5a in /ring-project/daemon/bin/dring+0x471cb5a
#14 0x555559c7b71a in /ring-project/daemon/bin/dring+0x472771a
#15 0x555559c943af in /ring-project/daemon/bin/dring+0x47403af
#16 0x555559d06102 in /ring-project/daemon/bin/dring+0x47b2102

previously allocated by thread T0 here:
#0 0x7ffff769eca1 in /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99
#1 0x55555a21b9e8 in /usr/include/c++/11.1.0/ext/new_allocator.h:121
#2 0x55555a1e4083 in /usr/include/c++/11.1.0/bits/alloc_traits.h:460
#3 0x55555a190197 in /usr/include/c++/11.1.0/bits/stl_vector.h:346
#4 0x55555a12af48 in /usr/include/c++/11.1.0/bits/vector.tcc:440
#5 0x55555a0e1a7c in /usr/include/c++/11.1.0/bits/vector.tcc:121
#6 0x55555a0b8c40 in /ring-project/daemon/src/call.h:286
#7 0x55555a8aaaaa in /ring-project/daemon/src/call.cpp:92
#8 0x55555abcb76d in /usr/include/c++/11.1.0/bits/sipcall.cpp:89
#9 0x55555a7c3341 in /usr/include/c++/11.1.0/ext/new_allocator.h:156
#10 0x55555a7c2185 in /usr/include/c++/11.1.0/bits/alloc_traits.h:512
#11 0x55555a7bfe6d in /usr/include/c++/11.1.0/bits/shared_ptr_base.h:519
#12 0x55555a7bcaa4 in /usr/include/c++/11.1.0/bits/shared_ptr_base.h:650
#13 0x55555a7b85e1 in /usr/include/c++/11.1.0/bits/shared_ptr_base.h:1337
#14 0x55555a7b2d2c in /usr/include/c++/11.1.0/bits/shared_ptr.h:409
#15 0x55555a7af189 in /usr/include/c++/11.1.0/bits/shared_ptr.h:861
#16 0x55555a7abce0 in /usr/include/c++/11.1.0/bits/shared_ptr.h:877
#17 0x55555a7a4782 in /ring-project/daemon/src/call_factory.cpp:54
#18 0x555559f39b16 in /usr/include/c++/11.1.0/bits/jamiaccount.cpp:445
#19 0x55555a838f0e in /ring-project/daemon/src/manager.cpp:3350
#20 0x55555a7f7aef in /ring-project/daemon/src/manager.cpp:1015
#21 0x555559d3c828 in /usr/include/c++/11.1.0/callmanager.cpp:67
#22 0x555559c70b5a in /ring-project/daemon/bin/dring+0x471cb5a
#23 0x555559c7b71a in /ring-project/daemon/bin/dring+0x472771a
#24 0x555559c943af in /ring-project/daemon/bin/dring+0x47403af
#25 0x555559d06102 in /ring-project/daemon/bin/dring+0x47b2102

Thread T1 created by T0 here:
(...)
#2 0x55555aaab6bd in /ring-project/daemon/src/scheduled_executor.cpp:27
#3 0x55555a7e61b3 in /ring-project/daemon/src/manager.cpp:456
#4 0x55555a7eea6c in /ring-project/daemon/src/manager.cpp:736
#5 0x55555a7ee39f in /ring-project/daemon/src/manager.cpp:711
#6 0x555559d3b25f in /ring-project/daemon/src/ring_api.cpp:57
#7 0x555559ae17db in /ring-project/daemon/bin/dring+0x458d7db
#8 0x555559ad1285 in /ring-project/daemon/bin/dring+0x457d285
#9 0x555559acf5e1 in /ring-project/daemon/bin/dring+0x457b5e1
#10 0x555559acf292 in /ring-project/daemon/bin/dring+0x457b292
#11 0x555559ace828 in /ring-project/daemon/bin/dring+0x457a828
#12 0x555559acdb01 in /ring-project/daemon/bin/dring+0x4579b01
#13 0x555559acd33f in /ring-project/daemon/bin/dring+0x457933f
#14 0x555559acbc8d in /ring-project/daemon/bin/dring+0x4577c8d
#15 0x555559aca91b in /ring-project/daemon/bin/dring+0x457691b
#16 0x555559ac8eec in /ring-project/daemon/bin/dring+0x4574eec
#17 0x555559ac693b in /ring-project/daemon/bin/dring+0x457293b
#18 0x7ffff380db24 in /usr/lib/libc.so.6+0x27b24

SUMMARY: AddressSanitizer: heap-use-after-free /ring-project/daemon/src/call.cpp:94 in operator()
Shadow bytes around the buggy address:
  0x0c068012a2d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068012a2e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068012a2f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068012a300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00
  0x0c068012a310: 00 00 fa fa fa fa fa fa fa fa 00 00 01 fa fa fa
=>0x0c068012a320:[fd]fd fd fd fa fa 00 00 00 07 fa fa fa fa fa fa
  0x0c068012a330: fa fa fd fd fd fd fa fa fd fd fd fa fa fa fd fd
  0x0c068012a340: fd fa fa fa 00 00 01 fa fa fa fa fa fa fa fa fa
  0x0c068012a350: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068012a360: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068012a370: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:   fa
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Container overflow:  fc
  Array cookie:ac
  Intra object redzone:bb
  ASan internal:   fe
  Left alloca redzone: ca
  Right alloca redzone:cb
  Shadow gap:  cc
==930034==ABORTING
--------------------------------------------------------------------------------

Change-Id: I23b4d1017b53a2d7fe224c92527254015e853168
GerritRingMirror pushed a commit that referenced this pull request Dec 7, 2021
Lock was:
    frame #3: 0x00000074f22d0b74 libc++_shared.so`std::__ndk1::mutex::lock()
    frame #5: 0x00000074f2f91b10 libring.so`std::__ndk1::lock_guard<std::__ndk1::mutex>::lock_guard()
    frame #6: 0x00000074f3156d3c libring.so`jami::video::SinkClient::registerTarget() at sinkclient.h:80:37
    frame #7: 0x00000074f3154f74 libring.so`DRing::registerSinkTarget(sinkId="2920319278288091") at videomanager.cpp:527:15
    frame #8: 0x00000074f2f1dccc libring.so`Java_net_jami_daemon_JamiServiceJNI_unregisterVideoCallback()
    frame #45: 0x00000074f2f3491c libring.so`SwigDirector_VideoCallback::decodingStopped()
    frame #48: 0x00000074f319a480 libring.so`void jami::emitSignal<DRing::VideoSignal::DecodingStopped>() at ring_signal.h:66:13
    frame #49: 0x00000074f31993ec libring.so`jami::video::SinkClient::setFrameSize(this=0x00000074c298caa0, width=0, height=0) at sinkclient.cpp:482:9
    frame #50: 0x00000074f3199f74 libring.so`jami::video::SinkClient::update() at sinkclient.cpp:427:13
    frame #51: 0x00000074f316c69c libring.so`jami::Observable<std::__ndk1::shared_ptr<DRing::MediaFrame> >::notify() at observer.h:138:23
    frame #52: 0x00000074f31a7094 libring.so`jami::video::VideoGenerator::publishFrame() at video_base.cpp:56:5


Change-Id: I1824bad767543a3e789e13af1489613a044a7473
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

Successfully merging this pull request may close these issues.

2 participants