/curl

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

curl_fuzzer: http2.c:1182: Assertion `0' failed. #2894

Closed
bagder opened this Issue Aug 17, 2018 · 0 comments

Comments

Assignees
No one assigned
Labels
HTTP/2
Projects
None yet
Milestone
No milestone
1 participant
@bagder
@bagder
Member

bagder commented Aug 17, 2018

I did this

This is a repost of a curl-fuzzer bug 9943. (closed for the public another 30 days)

Step #4: Running: /src/curl_fuzzer/corpora/curl_fuzzer/oss-fuzz-gen-5d7f57c3e59f01099cbb6fdb7aef7b15e847ca49
Step #4: curl_fuzzer: http2.c:1182: void Curl_http2_done(struct connectdata *, _Bool): Assertion `0' failed.
Step #4: ==8153== ERROR: libFuzzer: deadly signal

The assert was added in ac86eab to help out debugging the HTTP/2 issues in #2688.

curl/libcurl version

current 7.61.1-DEV git master

operating system

Linux, but not important

@bagder bagder added the HTTP/2 label Aug 17, 2018

bagder added a commit that referenced this issue Aug 17, 2018

@bagder
http2: add easymap layer to avoid stale pointers to freed memory 
Instead of passing the easy handle directly to nghttp2 to map from
stream_id to our structs, we create an 'easymap' node (for each new
stream) and add to a per-connection linked list. The easymap struct
itself contains a pointer to the easy handle and the stream id. This
node *MUST* remain in the list until the 'on_stream_close' callback
comes. This node can survive the easy handle.

If we want to remove the association between a stream and an easy handle
before the stream end callback from nghtp2, we now call
disassociate_easymap() which "soft removes" the association in the
easymap struct only.

This is necesary since when nghttp2_submit_request() is called, the
stream<=>easy association isn't immediately done by nghttp2 and if we
then try to clear the assication at once (using
nghttp2_session_set_stream_user_data), that clearing fails and we risk
having the (by now) old assication getting done by nghttp2 and then risk
subsequently fetching a stale pointer to already freed data.

Fixes #2688
Fixes #2894
Verified
This commit was signed with a verified signature.
@bagder bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2 Learn about signing commits
Loading status checks…
e5a6833

@bagder bagder referenced this issue Aug 17, 2018

Closed

http2: add easymap layer to avoid stale pointers to freed memory #2895

bagder added a commit that referenced this issue Aug 20, 2018

@bagder
http2: avoid set_stream_user_data() before stream is assigned 
... before the stream is started, we have it set to -1.

Fixes #2894
Verified
This commit was signed with a verified signature.
@bagder bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2 Learn about signing commits
Loading status checks…
1307081

@bagder bagder referenced this issue Aug 20, 2018

Closed

http2: avoid set_stream_user_data() before stream is assigned #2898

@bagder bagder closed this in e29ff2b Aug 21, 2018

xquery added a commit to xquery/curl that referenced this issue Sep 3, 2018

@bagder @xquery
http2: avoid set_stream_user_data() before stream is assigned 
... before the stream is started, we have it set to -1.

Fixes curl#2894
Closes curl#2898
933f9e5

falconindy added a commit to falconindy/curl that referenced this issue Sep 10, 2018

@bagder @falconindy
http2: avoid set_stream_user_data() before stream is assigned 
... before the stream is started, we have it set to -1.

Fixes curl#2894
Closes curl#2898
dd0a4b5

@lock lock bot locked as resolved and limited conversation to collaborators Nov 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.