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

Provide a destroy function for registered addresses #535

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

Conversation

sancane
Copy link

@sancane sancane commented 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:
#405
#147

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
@tuexen
Copy link
Member

tuexen commented Oct 11, 2020

Does it fix the issue for you? If that is the case, I can try to integrate this with some tweaks to minimise the impact on the kernel implementation...

@sancane
Copy link
Author

sancane commented Oct 11, 2020

It seems to perform well in all tests I've been doing. I'm sending this patch as proposal addressed to fix this problem in the hope that people more familiar with sctp code who are also suffering this issue can inspect it and look for possible side effects.
Honestly, I don't know usrsctp code very well and I don't understand much of what happens in there, so I'd rather someone with more knowledge than me could have a look at it. I would not like to break something unintentionally (usrsctp code is kind of complicated).
In fact, one of my main concerns is that, for example, if the sctp_ifap struct could be copied somewhere and I missed such deatils in my greps and the destroy function resuts being called more than once. Certainly I don't know if such a thing can happen, I did not see it when I was grepping the code

tbeloqui pushed a commit to pexip/gstreamer that referenced this pull request 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
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