Check for -1 after CURLINFO_LASTSOCKET#3993
Closed
alexcrichton wants to merge 1 commit into
Closed
Conversation
alexcrichton
force-pushed
the
fix-fault
branch
from
3d7ad6e to
defddcc
Compare
We're recently trying to upgrade to the current mater of libgit2 in Cargo but we're unfortunately hitting a segfault in one of our tests. A particular test is just a small smoke test that https works (e.g. it's configured in libgit2). This test attempts to clone from a URL which simply immediately drops connections after they're accepted (e.g. terminate abnormally). We expect to see a standard error from libgit2 but unfortunately we're seeing a segfault. This segfault is happening inside of the `wait_for` function of `curl_stream.c` at the line `FD_SET(fd, &errfd)` because `fd` is -1. This ends up doing an out-of-bounds array access that faults the program. I tracked back to where this -1 came from to the line here (returned by `CURLINFO_LASTSOCKET`) and added a check to return an error. This may not be the right fix though, so feedback is appreciated!
alexcrichton
force-pushed
the
fix-fault
branch
from
defddcc to
1d64dd7
Compare
pks-t
added a commit
that referenced
this pull request
Nov 11, 2016
Member
|
Thanks for the fix. I manually merged the commit via b782205 as I've removed the last sentence from the commit message (that is, the bit about not being sure if the fix is right, as it is). |
Contributor
Author
|
Thanks! |
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.
We're recently trying to upgrade to the current mater of libgit2 in Cargo but
we're unfortunately hitting a segfault in one of our tests. A particular test is
just a small smoke test that https works (e.g. it's configured in libgit2). This
test attempts to clone from a URL which simply immediately drops connections
after they're accepted (e.g. terminate abnormally). We expect to see a standard
error from libgit2 but unfortunately we're seeing a segfault.
This segfault is happening inside of the
wait_forfunction ofcurl_stream.cat the line
FD_SET(fd, &errfd)becausefdis -1. This ends up doing anout-of-bounds array access that faults the program. I tracked back to where this
-1 came from to the line here (returned by
CURLINFO_LASTSOCKET) and added acheck to return an error. This may not be the right fix though, so feedback is
appreciated!