Send callback is invoked on already closed socket. #405
Comments
|
I've finally completed bisect to find out the commit which introduced invocation of send callback on closed socket. view bisect.log (can be replayed with `git bisect replay < bisect.log`) |
|
This is still a problem for us. @tuexen, do you have any plan how to deal with this? Is there something we should be doing at the client level to wait for the socket to really be closed? Otherwise it seems like Would that pose a problem though? It would mean that when this happens, we wouldn't send out the abort chunk (that's what usrsctp is trying to do when the crash occurs). I guess delivery of that isn't guaranteed anyway though. |
So what is exactly the problem? When the user call |
|
Yes, the naïve assumption was that after calling And no, we're not using the upcall API. Should we be? Can you summarize how it works? This is what we're running into: Presumably this is from the INPKILL timer. |
|
Any updates on this? I'm experiencing the same behavior and I don't know how to handle it. Are callbacks really supposed to still be called after close? If so, how do you know when it is safe to assume no more calls? |
I just realise: You are not referring to the send callback in the socket API (upper layer interface), but to the function to send out packets (lower layer interface). Doesn't deregistering the address do what you want? |
|
Same here, it's indeed about the lower layer function to send out packets (the one passed to On my side the callback was sometimes called after |
Let me double check... |
|
Thanks. Actually the call might not be technically posterior to deregistering but simply concurrent, which results in the same issue depending on thread scheduling. |
sancane
added a commit
to sancane/usrsctp
that referenced
this issue
Oct 11, 2020
This patch tries to fix the problem of usrsctp calling callbacks even after the usrsctp_close function is invoked. Usrsctp seems to keep the asociation around until the teardown procedure is completed. The problem is that sometimes it is not possible to complete the shutdown procedure if the lower transport is gone like SCTP running on top of DTLS. In such case, callbacks triggered from usrsctp are providing an pointer address to an applications which could not be valid or dealocated. To let application know when they can safetely deallocate memory registered with an association, we are storing a destroy function which will be used for usrsctp to notify application when the association is gone. Related issues: sctplab#405 sctplab#147
sancane
added a commit
to sancane/usrsctp
that referenced
this issue
Oct 11, 2020
This patch tries to fix the problem of usrsctp calling callbacks even after the usrsctp_close function is invoked. Usrsctp seems to keep the asociation around until the teardown procedure is completed. The problem is that sometimes it is not possible to complete the shutdown procedure if the lower transport is gone like SCTP running on top of DTLS. In such case, callbacks triggered from usrsctp are providing an pointer address to an applications which could have been deallocated. To let applications know when they can safetely deallocate memory registered with an association, we are storing a destroy function which will be used for usrsctp to notify applications when the association is gone. Related issues: sctplab#405 sctplab#147
sancane
added a commit
to sancane/usrsctp
that referenced
this issue
Oct 11, 2020
This patch tries to fix the problem of usrsctp calling callbacks even after the usrsctp_close function is invoked. Usrsctp seems to keep the asociation around until the teardown procedure is completed. The problem is that sometimes it is not possible to complete the shutdown procedure if the lower transport is gone like SCTP running on top of DTLS. In such case, callbacks triggered from usrsctp are providing an pointer address to an applications which could have been deallocated. To let applications know when they can safely deallocate memory registered with an association, we are storing a destroy function which will be used for usrsctp to notify applications when the association is gone. Related issues: sctplab#405 sctplab#147
tbeloqui
pushed a commit
to pexip/gstreamer
that referenced
this issue
Mar 24, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread.
tbeloqui
pushed a commit
to pexip/gstreamer
that referenced
this issue
Mar 24, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread.
tbeloqui
pushed a commit
to pexip/gstreamer
that referenced
this issue
Mar 24, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread.
tbeloqui
pushed a commit
to pexip/gstreamer
that referenced
this issue
Mar 24, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
tbeloqui
pushed a commit
to pexip/gstreamer
that referenced
this issue
Aug 4, 2023
This patch tries to fix the problem of usrsctp calling callbacks even after the usrsctp_close function is invoked. Usrsctp seems to keep the asociation around until the teardown procedure is completed. The problem is that sometimes it is not possible to complete the shutdown procedure if the lower transport is gone like SCTP running on top of DTLS. In such case, callbacks triggered from usrsctp are providing an pointer address to an applications which could have been deallocated. To let applications know when they can safely deallocate memory registered with an association, we are storing a destroy function which will be used for usrsctp to notify applications when the association is gone. Related issues: sctplab/usrsctp#405 sctplab/usrsctp#147 From: sctplab/usrsctp#535
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
havardgraff
pushed a commit
to pexip/gstreamer
that referenced
this issue
Sep 27, 2023
Workaround for sctplab/usrsctp#405: - Since the sctp socket can outlive the sctp assoacition, we need to only register/deregister the association when creating/closing the socket. This prevents invalid calls to sctp_packet_out() and receive_cb() in potential invalid states. Workaround for sctplab/usrsctp#383: - Retry usrsctp_finish() for 5seconds. This fixes a race condition between usrsctp_close() and usrsctp_finish() in which a dead socket will be accessed by the SCTP thread. (cherry picked from commit 9cf8ac2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary:
usrsctpinvokes send callback on socket, which was already closed by user's code.Typically user code in send callback does some memory access to already freed memory.
This happens due to user's code assumption that send callback is never invoked after socket is closed with
usrsctp_close.Reproduces:
Sometimes
Versions:
I've detected that bug happen on current master, but it was not presented on version used by Chromium: 7a8bc9a from 22 Jun 2018The first commit with the issue is 6bafb12, merged in master as part of PR #249 on 9 Aug 2018.
Steps to reproduce (abstract):
usrsctpsocket withSO_LINGERset in non-blocking mode with send callback provided by user's code;usrsctpsocket withusrsctp_closeand release associated resources in user code;Expected result:
usrsctpwill never invoke send callback which is provided by user's code during socket initialization.Actual result:
Send callback is invoked for socket, which was previously closed with
usrsctp_close. It causes user's code misbehavior, typically accessing already freed memory, causing crash due toSIGSEGV.Steps to reproduce (concrete)
WebRTChas gtest which is accidentally able to reproduce send callback invocation on closed socket.fetch webrtc- fetchesWebRTCsource code;cd src- rest of commands assumed to be executed in WebRTC source code directory;gclient sync- checkout dependencies;gn gen out/ReleaseWithAsan --args='is_debug=false rtc_enable_sctp=true symbol_level=2 dcheck_always_on=true is_asan=true' --cflags="-fno-omit-frame-pointer -fno-sanitize-recover=all -fsanitize-address-use-after-scope"- generate build files with optimizations and address sanitizer enabled;autoninja -C out/ReleaseWithAsan rtc_media_unittests- compile unit tests;export ASAN_OPTIONS="handle_segv=1:detect_leaks=0:disable_coredump=0:unmap_shadow_on_exit=1:abort_on_error=1"configure ASAN options;export ASAN_SYMBOLIZER_PATH="$(pwd)/third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer"configure ASAN symbolizer;gdb --args ./out/ReleaseWithAsan/rtc_media_unittests --gtest_filter="*AllMessagesAreDeliveredOverLossyConnectionConcurrentTests" --gtest_also_run_disabled_tests- execute unittest with gdb attached.Investigation details:
It is occurred that
usrsctp_closemight not dispose/free socket.usrsctp_closeonly remove reference from "user's" code, basically doing decrement of referene counter.And if there are any references to
struct socket *the socket will surviveusrsctp_closeinvocation:usrsctp/usrsctplib/user_socket.c
Lines 2192 to 2194 in 09768bf
where
sorele:usrsctp/usrsctplib/user_socketvar.h
Lines 485 to 495 in 09768bf
so,
sofreeis only invoked when last reference removed.I've verified this by locally modifying
usrsctp_closeto have this:And I've got this assert triggered:
GDB backtrace of assert
For example,
WebRTCassumes there will no calls fromusrsctptoWebRTC:snipped from WebRTC source
with the assumption repeated here:
additional snipped from WebRTC source
Below the typical crash backtrace I've observed when send callback was invoked on already closed socket:
typical backtrace from GDB of crashed application
I've checked several crash dumps when send callback is invoked.
In all observed cases
sctp_handle_ticktriggered execution ofSCTP_TIMER_TYPE_HEARTBEATtimer, which in it's callback decided to close socket and invoke send callback:output from GDB
ASAN report on revision 6bafb12:
typical ASAN report produced on crash
There are several open question here:
usrsctp_closecompleted execution?0at the end ofusrsctp_closeinvocation, causing actual socket release?The text was updated successfully, but these errors were encountered: