Bug 185717 – [Curl] Crash due to broken Curl_easy handle since WinCairoRequirements v2018.05.16

• Component: Platform
• Status: RESOLVED FIXED

[Fujii Hironori]

[Curl] Crash due to broken Curl_easy handle since WinCairoRequirements v2018.05.16

After WinCairoRequirements v2018.05.16, crash happens in libcurl.
https://github.com/WebKitForWindows/WinCairoRequirements/releases/tag/v2018.05.16

1) Start MiniBrowser
2) Go to https://youtube.com/
3) Crash

Callstack:

> libcurl.dll!Curl_set_in_callback(Curl_easy * easy, bool value) Line 3109	C
> libcurl.dll!showit(Curl_easy * data, curl_infotype type, char * ptr, unsigned __int64 size) Line 811	C
> libcurl.dll!Curl_debug(Curl_easy * data, curl_infotype type, char * ptr, unsigned __int64 size, connectdata * conn) Line 874	C
> libcurl.dll!Curl_infof(Curl_easy * data, const char * fmt, ...) Line 245	C
> [Inline Frame] libcurl.dll!http2_connisdead(connectdata *) Line 214	C
> libcurl.dll!http2_conncheck(connectdata * check, unsigned int checks_to_perform) Line 236	C
> libcurl.dll!extract_if_dead(connectdata * conn, Curl_easy * data) Line 973	C
> libcurl.dll!ConnectionExists(Curl_easy * data, connectdata * needle, connectdata * * usethis, bool * force_reuse, bool * waitpipe) Line 1140	C
> libcurl.dll!create_conn(Curl_easy * data, connectdata * * in_connect, bool * async) Line 4401	C
> libcurl.dll!Curl_connect(Curl_easy * data, connectdata * * in_connect, bool * asyncp, bool * protocol_done) Line 4660	C
> libcurl.dll!multi_runsingle(Curl_multi * multi, curltime now, Curl_easy * data) Line 1421	C
> libcurl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Line 2165	C
> WebKit.dll!WebCore::CurlRequestScheduler::workerThread() Line 169	C++
> [Inline Frame] WebKit.dll!WebCore::CurlRequestScheduler::startThreadIfNeeded::__l5::<lambda_759a9b0dbc5d1b4468a3b240cce0f503>::operator()() Line 88	C++
> WebKit.dll!WTF::Function<void __cdecl(void)>::CallableWrapper<<lambda_759a9b0dbc5d1b4468a3b240cce0f503> >::call() Line 101	C++
> [Inline Frame] WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 56	C++
> WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) Line 136	C++
> WTF.dll!WTF::wtfThreadEntryPoint(void * data) Line 156	C++
> [External Code]	


Callstack:

> [Inline Frame] libcurl.dll!h2_pri_spec(Curl_easy *) Line 1440	C
> libcurl.dll!h2_session_send(Curl_easy * data, nghttp2_session * h2) Line 1466	C
> libcurl.dll!h2_process_pending_input(connectdata * conn, http_conn * httpc, CURLcode * err) Line 1304	C
> [Inline Frame] libcurl.dll!http2_connisdead(connectdata *) Line 219	C
> libcurl.dll!http2_conncheck(connectdata * check, unsigned int checks_to_perform) Line 236	C
> libcurl.dll!extract_if_dead(connectdata * conn, Curl_easy * data) Line 973	C
> libcurl.dll!call_extract_if_dead(connectdata * conn, void * param) Line 1003	C
> libcurl.dll!Curl_conncache_foreach(Curl_easy * data, conncache * connc, void * param, int(*)(connectdata *, void *) func) Line 382	C
> [Inline Frame] libcurl.dll!prune_dead_connections(Curl_easy *) Line 1025	C
> libcurl.dll!create_conn(Curl_easy * data, connectdata * * in_connect, bool * async) Line 4382	C
> libcurl.dll!Curl_connect(Curl_easy * data, connectdata * * in_connect, bool * asyncp, bool * protocol_done) Line 4660	C
> libcurl.dll!multi_runsingle(Curl_multi * multi, curltime now, Curl_easy * data) Line 1421	C
> libcurl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Line 2165	C
> WebKit.dll!WebCore::CurlRequestScheduler::workerThread() Line 169	C++
> [Inline Frame] WebKit.dll!WebCore::CurlRequestScheduler::startThreadIfNeeded::__l5::<lambda_759a9b0dbc5d1b4468a3b240cce0f503>::operator()() Line 88	C++
> WebKit.dll!WTF::Function<void __cdecl(void)>::CallableWrapper<<lambda_759a9b0dbc5d1b4468a3b240cce0f503> >::call() Line 101	C++
> [Inline Frame] WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 56	C++
> WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) Line 136	C++
> WTF.dll!WTF::wtfThreadEntryPoint(void * data) Line 156	C++
> [External Code]

[Fujii Hironori]

It seems that elements of `conn_list` of `connectbundle` are broken.

[Fujii Hironori]

I have confirmed this crash can be solved by replacing libcurl.dll with one disabled USE_NGHTTP2.
This is a critical issue, USE_NGHTTP2 should be disabled until it will be addressed.

[Basuke Suzuki]

I'll look into this today, thank you for reporting.

[Fujii Hironori]

I wouldn't see such crashes if I replaced with libcurl.dll built by me without any modificaitons.

Don, check it in your env.

[Basuke Suzuki]

Can you give me your build option? Or is that same with WinCairoRequirements? I can reproduce this crash with libs build by myself locally using vcpkg.exe.

[Fujii Hironori]

Thanks for confirming, Basuke. It's my mistake. 
I should invoke `vcpkg install curl[ssl]` instead of `vcpkg install curl`.

[Fujii Hironori]

A connectdata stored in a conncache holds a deleted Curl_easy.
http2_connisdead accesses the deleted Curl_easy.

[Basuke Suzuki]

Wait a minute. That's reminds me my patch to the curl. https://github.com/curl/curl/pull/2221/files

I'll take a look inside it.

[Basuke Suzuki]

No, my patch doesn't related with this crash. Sigh.

[Fujii Hironori]

url: fix dangling conn->data pointer · curl/curl@2c15693
https://github.com/curl/curl/commit/2c15693a3c355d8296a1828123a864397296460b

[WIP] Avoid using free'd easy handles referenced from connection cache by dtzWill · Pull Request #2669 · curl/curl
https://github.com/curl/curl/pull/2669

I haven't tried it yet. But, sounds promising.

[Don Olmstead]

(In reply to Fujii Hironori from comment #10)
> url: fix dangling conn->data pointer · curl/curl@2c15693
> https://github.com/curl/curl/commit/2c15693a3c355d8296a1828123a864397296460b
> 
> [WIP] Avoid using free'd easy handles referenced from connection cache by
> dtzWill · Pull Request #2669 · curl/curl
> https://github.com/curl/curl/pull/2669
> 
> I haven't tried it yet. But, sounds promising.

https://github.com/WebKitForWindows/WinCairoRequirements/releases/tag/v2018.07.03

That has the commit referenced. It appears there might be more problems there but it should be a start.

[Don Olmstead]

(In reply to Don Olmstead from comment #11)
> (In reply to Fujii Hironori from comment #10)
> > url: fix dangling conn->data pointer · curl/curl@2c15693
> > https://github.com/curl/curl/commit/2c15693a3c355d8296a1828123a864397296460b
> > 
> > [WIP] Avoid using free'd easy handles referenced from connection cache by
> > dtzWill · Pull Request #2669 · curl/curl
> > https://github.com/curl/curl/pull/2669
> > 
> > I haven't tried it yet. But, sounds promising.
> 
> https://github.com/WebKitForWindows/WinCairoRequirements/releases/tag/v2018.
> 07.03
> 
> That has the commit referenced. It appears there might be more problems
> there but it should be a start.

There seems to be more crashing after the SSL cert patch. There is also some work in https://github.com/curl/curl/issues/2674 that looks like it might be in the ballpark of the crashes I'm seeing in the debug build.

Hopefully they have a patch this week. We should probably roll to that or request a release from them including the changes since 7.60.0.

[Fujii Hironori]

WinCairoRequirements.zip v2018.07.03 (the second release)

1) Start MiniBrowser
2) Open WebKitLegacy Window
2) Go to https://youtube.com/
3) Crash

Callstack:

> libcurl.dll!Curl_conncache_bundle_size(connectdata * conn) Line 206	C
> libcurl.dll!create_conn(Curl_easy * data, connectdata * * in_connect, bool * async) Line 4407	C
> libcurl.dll!Curl_connect(Curl_easy * data, connectdata * * in_connect, bool * asyncp, bool * protocol_done) Line 4660	C
> libcurl.dll!multi_runsingle(Curl_multi * multi, curltime now, Curl_easy * data) Line 1421	C
> libcurl.dll!curl_multi_perform(Curl_multi * multi, int * running_handles) Line 2165	C
> WebKit.dll!WebCore::CurlRequestScheduler::workerThread() Line 169	C++
> [Inline Frame] WebKit.dll!WebCore::CurlRequestScheduler::startThreadIfNeeded::__l5::<lambda_759a9b0dbc5d1b4468a3b240cce0f503>::operator()() Line 88	C++
> WebKit.dll!WTF::Function<void __cdecl(void)>::CallableWrapper<<lambda_759a9b0dbc5d1b4468a3b240cce0f503> >::call() Line 101	C++
> [Inline Frame] WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 56	C++
> WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext) Line 136	C++
> WTF.dll!WTF::wtfThreadEntryPoint(void * data) Line 156	C++
> [External Code]

[Fujii Hironori]

Fixed in the latest WinCairoRequirements.
https://github.com/WebKitForWindows/WinCairoRequirements/releases/tag/v2018.08.01