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

Address Sanitizer Issue, Fix Stack buffer reference after it has gone… #3034

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jlsantiago0
Copy link
Contributor

@jlsantiago0 jlsantiago0 commented Oct 3, 2024

Address Sanitizer Issue, Fix Stack buffer reference after it has gone out of scope.


==1873251==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fe0a779a88c at pc 0x000001c942ad bp 0x7fe0a876c9c0 sp 0x7fe0a876c9b8
READ of size 4 at 0x7fe0a779a88c thread T17
    #0 0x1c942ac in srt::CChannel::getTargetAddress(msghdr const&) const /mnt/jlsws2/dev/orthrus/master/src/vendor/utils/haisrt/srt-1.5.4-rc.1/srtcore/channel.h:218:57
    #1 0x1c9384a in srt::CChannel::recvfrom(srt::sockaddr_any&, srt::CPacket&) const /mnt/jlsws2/dev/orthrus/master/src/vendor/utils/haisrt/srt-1.5.4-rc.1/srtcore/channel.cpp:944:31
    #2 0x1dd9c3d in srt::CRcvQueue::worker_RetrieveUnit(int&, srt::CUnit*&, srt::sockaddr_any&) /mnt/jlsws2/dev/orthrus/master/src/vendor/utils/haisrt/srt-1.5.4-rc.1/srtcore/queue.cpp:1407:35
    #3 0x1dd8817 in srt::CRcvQueue::worker(void*) /mnt/jlsws2/dev/orthrus/master/src/vendor/utils/haisrt/srt-1.5.4-rc.1/srtcore/queue.cpp:1246:43
    #4 0x7fe0d5fbc668 in asan_thread_start(void*) ../../../llvm-project-llvmorg-18.1.8/compiler-rt/lib/asan/asan_interceptors.cpp:239:28
    #5 0x7fe0c36a339c in start_thread /usr/src/debug/glibc/glibc/nptl/pthread_create.c:447:8
    #6 0x7fe0c372849b in __GI___clone3 /usr/src/debug/glibc/glibc/misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

Address 0x7fe0a779a88c is located in stack of thread T17 at offset 140 in frame
    #0 0x1c9357f in srt::CChannel::recvfrom(srt::sockaddr_any&, srt::CPacket&) const /mnt/jlsws2/dev/orthrus/master/src/vendor/utils/haisrt/srt-1.5.4-rc.1/srtcore/channel.cpp:841

  This frame has 3 object(s):
    [32, 88) 'mh' (line 865)
    [128, 216) 'mh_crtl_buf' (line 881) <== Memory access at offset 140 is inside this variable
    [256, 288) 'ref.tmp' (line 944)

The stack variable object mh after call to recvmsg will reference data contained in the stack buffer mh_crtl_buf . This data is then referenced by a later call to getTargetAddress(mh), but the stack buffer has already gone out of scope. This PR moves the stack variable outside of the conditional block scope so that it is still in scope while it is being referenced later on in this method.

Issue uncovered by using the CLANG Address Sanitizer.

Comment on lines +867 to +872
#ifdef SRT_ENABLE_PKTINFO
// ASAN, Data in this buffer is referenced by mh and needs to remain in
// scope while it is being used. It needs to stay in scope because it is
// later referenced below via getTargetAddress(mh).
char mh_crtl_buf[sizeof(CMSGNodeIPv4) + sizeof(CMSGNodeIPv6)];
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifdef SRT_ENABLE_PKTINFO
// ASAN, Data in this buffer is referenced by mh and needs to remain in
// scope while it is being used. It needs to stay in scope because it is
// later referenced below via getTargetAddress(mh).
char mh_crtl_buf[sizeof(CMSGNodeIPv4) + sizeof(CMSGNodeIPv6)];
#endif
#ifdef SRT_ENABLE_PKTINFO
// This buffer is mounted inside mh so it must stay in the same scope
char mh_crtl_buf[sizeof(CMSGNodeIPv4) + sizeof(CMSGNodeIPv6)];
#endif

Comment on lines +887 to +889

// ASAN: Moved outside of this block scope, see above.
//char mh_crtl_buf[sizeof(CMSGNodeIPv4) + sizeof(CMSGNodeIPv6)];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ASAN: Moved outside of this block scope, see above.
//char mh_crtl_buf[sizeof(CMSGNodeIPv4) + sizeof(CMSGNodeIPv6)];

Would be enough if you describe the needs in the comments. This can be simply removed.

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