Open
Conversation
jp-sarte
force-pushed
the
bugfix/segfault-in-encode-cleanup
branch
2 times, most recently
from
b695675 to
949ca76
Compare
cyanreg
reviewed
Dec 12, 2025
| pthread_join(ctx->thread, NULL); | ||
| } | ||
| if (ctx->mutex_status != MUTEX_STATUS_NOT_INITIALIZED) { | ||
| pthread_mutex_destroy(&ctx->lock); |
Owner
There was a problem hiding this comment.
You can just check if it exists by checking its value, you know?
Author
There was a problem hiding this comment.
I presume that you mean ctx->thread here, which you're correct about. I didn't know about that! Thanks.
Based on my research though it doesn't look like it's a good idea to rely on a zero-value for whether ctx->lock is valid or not, since a valid lock might actually just be zero. See PTHREAD_MUTEX_INITIALIZER at https://elixir.bootlin.com/glibc/glibc-2.29/source/sysdeps/nptl/pthread.h
This fixes a potential segfault when `avio_open` fails -> we go to cleanup -> cleanup sees that `s->mutex_held` (now `s->mutex_status`) is true, and thus tries to acquire the mutex, yet the mutex isn't initialized: segfault. A similar issue occurs when we try to `pthread_join` on the thread in the cleanup if it's not created. Thus a "bool" for the initialization of the thread is added. One condition that `avio_open` can fail on is when the file path is too long.
jp-sarte
force-pushed
the
bugfix/segfault-in-encode-cleanup
branch
from
949ca76 to
a7ed86f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a potential segfault when
avio_openfails -> we go to cleanup -> cleanup sees thats->mutex_held(nows->mutex_status) is true, and thus tries to acquire the mutex, yet the mutex isn't initialized: segfault.A similar issue occurs when we try to
pthread_joinon the thread in the cleanup if it's not created. Thus a "bool" for the initialization of the thread is added.One condition that
avio_opencan fail on is when the file path is too long.This is most likely the segfault experienced in #110