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

segfault when Hyper/Rustls client used with Git #6619

Closed
kevinburke1 opened this issue Feb 17, 2021 · 5 comments
Closed

segfault when Hyper/Rustls client used with Git #6619

kevinburke1 opened this issue Feb 17, 2021 · 5 comments

Comments

@kevinburke1
Copy link

I'm compiling the Hyper C bindings into libcurl, and then building git with this libcurl attached. Unfortunately I'm getting a segfault when I try to use the resulting git binaries.

Steps to reproduce:

  • install Git with Hyper+Rustls support (I set this up for Macs at brew install meterup/safe/git if you want to give it a try).

    curl 7.76.0-DEV (x86_64-apple-darwin19.6.0) libcurl/7.76.0-DEV SecureTransport (crustls/0.2.0/rustls/0.19.0) zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 libidn2/2.3.0 nghttp2/1.43.0 librtmp/2.3 Hyper/0.14.4
    Release-Date: [unreleased]
    Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp smb smbs smtp smtps telnet tftp
    Features: alt-svc AsynchDNS brotli Debug GSS-API HTTP2 IDN IPv6 Kerberos Largefile libz Metalink MultiSSL NTLM NTLM_WB SPNEGO SSL TrackMemory UnixSockets zstd
    
  • Try to run git fetch https://go.googlesource.com/go

Running with GIT_TRACE=1 GIT_CURL_VERBOSE=1 I get the following error message:

$ GIT_TRACE=1 GIT_CURL_VERBOSE=1 git fetch https://go.googlesource.com/go
12:23:39.274689 git.c:444               trace: built-in: git fetch https://go.googlesource.com/go
12:23:39.275893 run-command.c:664       trace: run_command: GIT_DIR=.git git remote-https https://go.googlesource.com/go https://go.googlesource.com/go
12:23:39.279962 git.c:730               trace: exec: git-remote-https https://go.googlesource.com/go https://go.googlesource.com/go
12:23:39.280388 run-command.c:664       trace: run_command: git-remote-https https://go.googlesource.com/go https://go.googlesource.com/go
12:23:39.291894 http.c:756              == Info: STATE: INIT => CONNECT handle 0x7fb6c4011808; line 1621 (connection #-5000)
12:23:39.292473 http.c:756              == Info: Couldn't find host go.googlesource.com in the (nil) file; using defaults
12:23:39.292489 http.c:756              == Info: Added connection 0. The cache now contains 1 members
12:23:39.292556 http.c:756              == Info: STATE: CONNECT => RESOLVING handle 0x7fb6c4011808; line 1667 (connection #0)
12:23:39.295617 http.c:756              == Info: family0 == v4, family1 == v6
12:23:39.295674 http.c:756              == Info:   Trying 74.125.195.82:443...
12:23:39.295773 http.c:756              == Info: STATE: RESOLVING => CONNECTING handle 0x7fb6c4011808; line 1749 (connection #0)
12:23:39.325505 http.c:756              == Info: Connected to go.googlesource.com (74.125.195.82) port 443 (#0)
12:23:39.325533 http.c:756              == Info: STATE: CONNECTING => PROTOCONNECT handle 0x7fb6c4011808; line 1814 (connection #0)
12:23:39.329935 http.c:756              == Info: ALPN, offering http/1.1
12:23:39.330091 http.c:756              == Info: STATE: PROTOCONNECT => PROTOCONNECTING handle 0x7fb6c4011808; line 1832 (connection #0)
12:23:39.398824 http.c:756              == Info: TLS 1.2 connection using TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
12:23:39.399145 http.c:756              == Info: Server certificate: *.googlecode.com
12:23:39.399284 http.c:756              == Info: Server certificate: GTS CA 1O1
12:23:39.399364 http.c:756              == Info: Server certificate: GlobalSign
12:23:39.399378 http.c:756              == Info: STATE: PROTOCONNECTING => DO handle 0x7fb6c4011808; line 1851 (connection #0)
12:23:39.399391 http.c:756              == Info: Time for the Hyper dance
12:23:39.399685 http.c:703              => Send header, 0000000052 bytes (0x00000034)
12:23:39.399697 http.c:715              => Send header: GET /go/info/refs?service=git-upload-pack HTTP/1.1
12:23:39.399748 http.c:703              => Send header, 0000000027 bytes (0x0000001b)
12:23:39.399757 http.c:715              => Send header: Host: go.googlesource.com
12:23:39.399766 http.c:703              => Send header, 0000000024 bytes (0x00000018)
12:23:39.399771 http.c:715              => Send header: User-Agent: git/2.30.1
12:23:39.399778 http.c:703              => Send header, 0000000013 bytes (0x0000000d)
12:23:39.399784 http.c:715              => Send header: Accept: */*
12:23:39.399811 http.c:703              => Send header, 0000000098 bytes (0x00000062)
12:23:39.399818 http.c:715              => Send header: Cookie: o=<redacted>
12:23:39.399828 http.c:703              => Send header, 0000000033 bytes (0x00000021)
12:23:39.399833 http.c:715              => Send header: Accept-Language: en-US, *;q=0.9
12:23:39.399841 http.c:703              => Send header, 0000000018 bytes (0x00000012)
12:23:39.399846 http.c:715              => Send header: Pragma: no-cache
12:23:39.399874 http.c:703              => Send header, 0000000025 bytes (0x00000019)
12:23:39.399880 http.c:715              => Send header: Git-Protocol: version=2
12:23:39.399886 http.c:703              => Send header, 0000000002 bytes (0x00000002)
12:23:39.399891 http.c:715              => Send header:
12:23:39.400079 http.c:756              == Info: STATE: DO => DID handle 0x7fb6c4011808; line 1909 (connection #0)
12:23:39.400134 http.c:756              == Info: STATE: DID => PERFORMING handle 0x7fb6c4011808; line 2028 (connection #0)
12:23:39.889983 http.c:756              == Info: HTTP 1.1 or later with persistent connection
12:23:39.890025 http.c:703              <= Recv header, 0000000017 bytes (0x00000011)
12:23:39.890033 http.c:715              <= Recv header: HTTP/1.1 200 OK
error: git-remote-https died of signal 11

I tried running with RUST_BACKTRACE=1 but that didn't yield anything useful.

Here's the crash file for this segfault. I have a core dump but am having trouble getting gdb to be able to open it to inspect it.

$ cat ~/Library/Logs/DiagnosticReports/git-remote-http_2021-02-17-122340_HYENA-2.crash
Process:               git-remote-http [41576]
Path:                  /usr/local/Cellar/git/2.30.1/libexec/git-core/git-remote-http
Identifier:            git-remote-http
Version:               ???
Code Type:             X86-64 (Native)
Parent Process:        ??? [41575]
Responsible:           iTerm2 [40587]
User ID:               501

Date/Time:             2021-02-17 12:23:39.890 -0800
OS Version:            Mac OS X 10.15.7 (19H512)
Report Version:        12
Anonymous UUID:        D2590148-2569-50FD-C193-1A6BC086FB3F

Sleep/Wake UUID:       44375010-5910-4365-9D3B-44071C6D5A10

Time Awake Since Boot: 300000 seconds
Time Since Wake:       11000 seconds

System Integrity Protection: enabled

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000008
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Segmentation fault: 11
Termination Reason:    Namespace SIGNAL, Code 0xb
Terminating Process:   exc handler [41576]

VM Regions Near 0x8:
-->
    __TEXT                 0000000103ec4000-000000010401c000 [ 1376K] r-x/r-x SM=COW  /usr/local/Cellar/git/2.30.1/libexec/git-core/git-remote-http

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   git-remote-https              	0x0000000103fa6b58 strbuf_grow + 16
1   git-remote-https              	0x0000000103fa7020 strbuf_add + 27
2   git-remote-https              	0x0000000103ecb6d5 fwrite_buffer + 27
3   libcurl.4.dylib               	0x00000001040b1fba Curl_hyper_stream + 594
4   libcurl.4.dylib               	0x00000001040f5df8 Curl_readwrite + 203
5   libcurl.4.dylib               	0x00000001040dfa80 multi_runsingle + 2113
6   libcurl.4.dylib               	0x00000001040df1c6 curl_multi_perform + 113
7   git-remote-https              	0x0000000103ecd4cb step_active_slots + 25
8   git-remote-https              	0x0000000103ecd532 run_active_slot + 67
9   git-remote-https              	0x0000000103ecd7fd run_one_slot + 41
10  git-remote-https              	0x0000000103ecfb06 http_request + 1630
11  git-remote-https              	0x0000000103ecd9b7 http_request_reauth + 34
12  git-remote-https              	0x0000000103eca131 discover_refs + 625
13  git-remote-https              	0x0000000103ec93fc cmd_main + 4484
14  git-remote-https              	0x0000000103ed0e55 main + 130
15  libdyld.dylib                 	0x00007fff6dd79cc9 start + 1

Thread 1:
0   libsystem_pthread.dylib       	0x00007fff6df79b68 start_wqthread + 0

Thread 2:
0   libsystem_pthread.dylib       	0x00007fff6df79b68 start_wqthread + 0

Thread 0 crashed with X86 Thread State (64-bit):
  rax: 0x00007fb6c4804218  rbx: 0x0000000000000000  rcx: 0x0000000000000000  rdx: 0x0000000000000011
  rdi: 0x0000000000000000  rsi: 0x0000000000000011  rbp: 0x00007ffeebd3a720  rsp: 0x00007ffeebd3a710
   r8: 0x0000000000000059   r9: 0x0000000000000035  r10: 0x00007fb6c4800000  r11: 0x0000000000000000
  r12: 0x00000000000000c8  r13: 0x00000000000000c8  r14: 0x0000000000000000  r15: 0x00007fb6c4804218
  rip: 0x0000000103fa6b58  rfl: 0x0000000000010217  cr2: 0x0000000000000008

Logical CPU:     6
Error Code:      0x00000004 (no mapping for user data read)
Trap Number:     14

See also rustls/rustls-ffi#49, hyperium/hyper#2438 (I wasn't sure which place to submit it.) Hyper suggested the issue may be in curl itself.

@bagder
Copy link
Member

bagder commented Feb 17, 2021

Unfortunately we still have a fair amount of tests cases that fail with the Hyper backend, and perhaps one of those is this problem. Can you make this appear without using git, perhaps with using the curl tool against the same URL or something? Can you make a build with debug symbols and perhaps use lldb (which always seems to be easier to get going on macOS) to investigate more specific details around the crash and local variables in that stack frame?

@kevinburke1
Copy link
Author

Can you make this appear without using git, perhaps with using the curl tool against the same URL or something?

I can't reproduce with only curl, at least, I tried sending exactly the same headers as were sent in the debug log and I can't get a segfault.

I compiled with --enable-debug set to true but have never used lldb before so will give that a try.

@kevinburke1
Copy link
Author

kevinburke1 commented Feb 17, 2021

Okay, I figured out how to start a lldb session that's live at the time of the segfault, but not sure what to do next.

Process 9264 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
    frame #0: 0x00000001000e2b58 git-remote-https`strbuf_grow + 16
git-remote-https`strbuf_grow:
->  0x1000e2b58 <+16>: movq   0x8(%rdi), %rax
    0x1000e2b5c <+20>: movq   %rax, %rcx
    0x1000e2b5f <+23>: notq   %rcx
    0x1000e2b62 <+26>: cmpq   %rsi, %rcx
Target 0: (git-remote-https) stopped.
(lldb) thread backtrace
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x00000001000e2b58 git-remote-https`strbuf_grow + 16
    frame #1: 0x00000001000e3020 git-remote-https`strbuf_add + 27
    frame #2: 0x00000001000076d5 git-remote-https`fwrite_buffer + 27
    frame #3: 0x00000001002ebfba libcurl.4.dylib`Curl_hyper_stream + 594
    frame #4: 0x000000010032fdf8 libcurl.4.dylib`Curl_readwrite + 203
    frame #5: 0x0000000100319a80 libcurl.4.dylib`multi_runsingle + 2113
    frame #6: 0x00000001003191c6 libcurl.4.dylib`curl_multi_perform + 113
    frame #7: 0x00000001000094cb git-remote-https`step_active_slots + 25
    frame #8: 0x0000000100009532 git-remote-https`run_active_slot + 67
    frame #9: 0x00000001000097fd git-remote-https`run_one_slot + 41
    frame #10: 0x000000010000bb06 git-remote-https`http_request + 1630
    frame #11: 0x00000001000099b7 git-remote-https`http_request_reauth + 34
    frame #12: 0x0000000100006131 git-remote-https`discover_refs + 625
    frame #13: 0x00000001000053fc git-remote-https`cmd_main + 4484
    frame #14: 0x000000010000ce55 git-remote-https`main + 130
    frame #15: 0x00007fff699fbcc9 libdyld.dylib`start + 1
    frame #16: 0x00007fff699fbcc9 libdyld.dylib`start + 1
(lldb) v
(lldb) var
(lldb) register read
General Purpose Registers:
       rax = 0x0000000102904748
       rbx = 0x0000000000000000
       rcx = 0x0000000000000000
       rdx = 0x0000000000000011
       rdi = 0x0000000000000000
       rsi = 0x0000000000000011
       rbp = 0x00007ffeefbfe700
       rsp = 0x00007ffeefbfe6f0
        r8 = 0x0000000000000000
        r9 = 0x0000000000000000
       r10 = 0x0000000000010000
       r11 = 0xffff800212d07731
       r12 = 0x00000000000000c8
       r13 = 0x00000000000000c8
       r14 = 0x0000000000000000
       r15 = 0x0000000102904748
       rip = 0x00000001000e2b58  git-remote-https`strbuf_grow + 16
    rflags = 0x0000000000010217
        cs = 0x000000000000002b
        fs = 0x0000000000000000
        gs = 0x0000000000000000

Last bit of code in Hyper, arrow points to the line that's executing at the time of the segfault.

    0x1002ebf75 <+525>:  callq  0x1002f7cb9               ; Curl_dyn_ptr
    0x1002ebf7a <+530>:  movq   %r14, %rdi
    0x1002ebf7d <+533>:  movl   $0x1, %esi
    0x1002ebf82 <+538>:  movq   %rax, %rdx
    0x1002ebf85 <+541>:  movq   %rbx, %rcx
    0x1002ebf88 <+544>:  callq  0x100320273               ; Curl_debug
    0x1002ebf8d <+549>:  movq   %r14, %rdi
    0x1002ebf90 <+552>:  movl   $0x1, %esi
    0x1002ebf95 <+557>:  callq  0x10031b17e               ; Curl_set_in_callback
    0x1002ebf9a <+562>:  movq   %r15, %rdi
    0x1002ebf9d <+565>:  callq  0x1002f7cb9               ; Curl_dyn_ptr
    0x1002ebfa2 <+570>:  movq   0x1f0(%r14), %rcx
    0x1002ebfa9 <+577>:  movl   $0x1, %esi
    0x1002ebfae <+582>:  movq   %rax, %rdi
    0x1002ebfb1 <+585>:  movq   %rbx, %rdx
    0x1002ebfb4 <+588>:  callq  *-0x160(%rbp)
->  0x1002ebfba <+594>:  movq   %rax, %r15
    0x1002ebfbd <+597>:  movq   %r14, %rdi
    0x1002ebfc0 <+600>:  xorl   %esi, %esi
    0x1002ebfc2 <+602>:  callq  0x10031b17e               ; Curl_set_in_callback

strbuf_grow

git-remote-https`strbuf_grow:
    0x1000e2b48 <+0>:   pushq  %rbp
    0x1000e2b49 <+1>:   movq   %rsp, %rbp
    0x1000e2b4c <+4>:   pushq  %r14
    0x1000e2b4e <+6>:   pushq  %rbx
    0x1000e2b4f <+7>:   cmpq   $-0x1, %rsi
    0x1000e2b53 <+11>:  je     0x1000e2bb1               ; <+105>
    0x1000e2b55 <+13>:  movq   %rdi, %r14
->  0x1000e2b58 <+16>:  movq   0x8(%rdi), %rax
    0x1000e2b5c <+20>:  movq   %rax, %rcx
    0x1000e2b5f <+23>:  notq   %rcx
    0x1000e2b62 <+26>:  cmpq   %rsi, %rcx

@jon-zu
Copy link

jon-zu commented Feb 21, 2021

Well It seems rdi is 0 so after some research I noticed that for fwritebuf userdata should be set to strbuf with CURLOPT_WRITEDATA. Maybe the error is here: https://github.com/git/git/blob/master/remote-curl.c#L858 not sure how CURLOPT_FILE behaves here.

@bagder
Copy link
Member

bagder commented Mar 11, 2021

It will be helpful if you build with debug symbols present and perhaps run it with a debugger and check the local situation in the curl code at the position of the crash.

Or perhaps run the curl test suite instead and help us fix the remaining test failures with Hyper as before they run fine, we can assume the problems are our own. There are still over a hundred failing test cases with Hyper.

I'm not sure this issue is worth keeping open right now.

@bagder bagder closed this as completed Mar 12, 2021
kevinburke added a commit to kevinburke/curl that referenced this issue Apr 26, 2021
Previously if a caller set CURLOPT_WRITEFUNCTION but did not set a
CURLOPT_HEADERDATA buffer, Hyper would still attempt to write headers
to the data->set.writeheader header buffer, even though it is null.
This led to NPE segfaults attempting to use libcurl+Hyper with Git,
for example.

Instead, process the client write for the status line using the same
logic we use to process the client write for the later HTTP headers,
which contains the appropriate guard logic. As a side benefit,
data->set.writeheader is now only read in one file instead of two.

Fixes curl#6619.
Fixes rustls/rustls-ffi#49.
Fixes hyperium/hyper#2438.
kevinburke added a commit to kevinburke/curl that referenced this issue Apr 26, 2021
Previously if a caller set CURLOPT_WRITEFUNCTION but did not set a
CURLOPT_HEADERDATA buffer, Hyper would still attempt to write headers
to the data->set.writeheader header buffer, even though it is null.
This led to NPE segfaults attempting to use libcurl+Hyper with Git,
for example.

Instead, process the client write for the status line using the same
logic we use to process the client write for the later HTTP headers,
which contains the appropriate guard logic. As a side benefit,
data->set.writeheader is now only read in one file instead of two.

Fixes curl#6619.
Fixes rustls/rustls-ffi#49.
Fixes hyperium/hyper#2438.
bagder pushed a commit that referenced this issue Apr 27, 2021
Previously if a caller set CURLOPT_WRITEFUNCTION but did not set a
CURLOPT_HEADERDATA buffer, Hyper would still attempt to write headers to
the data->set.writeheader header buffer, even though it is null.  This
led to NPE segfaults attempting to use libcurl+Hyper with Git, for
example.

Instead, process the client write for the status line using the same
logic we use to process the client write for the later HTTP headers,
which contains the appropriate guard logic. As a side benefit,
data->set.writeheader is now only read in one file instead of two.

Fixes #6619
Fixes rustls/rustls-ffi#49
Fixes hyperium/hyper#2438
Closes #6971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants