Closed iiibui closed 2 years ago
Can you say how you got or installed libcurl 7.80.0-DEV?
Can you say how you got or installed libcurl 7.80.0-DEV?
Build from source:
git clone --depth=1 https://github.com/curl/curl.git
cd curl
autoreconf -fi
./configure --prefix=/apps/svr/curl --with-openssl=/apps/svr/openssl --with-nghttp2=/apps/svr/nghttp2
make && make install
I found that if remove the http2 support, it won't crash, and also I can't reproduce this problem on my Mac and Windows, maybe I should compare the code of libcurl-7.73.0 and libcurl 7.80.0-DEV. Any way, I think it's better to remove all EasyHandle from Multi before Multi cleaning up(by curl_multi_cleanup).
You're right, it does seem that the curl docs recommend removing all easy handles from a multi handle first before freeing the multi handle: https://curl.se/libcurl/c/curl_multi_cleanup.html. Though it is unclear if this is related to this issue or not, or if the way we're currently doing it is strictly incorrect.
I also wouldn't rule out the possibility that this is an upstream issue in either curl or OpenSSL.
I'll see if I am able to reproduce and if so, whether removing all handles first corrects the problem.
So far I am not able to reproduce this on my local machine or inside a Centos 7.3.1611 Docker container. Are you also building OpenSSL and nghttp2 yourself? I see you are pointing to nonstandard paths for those. It would be helpful to see how you are building those.
It might also help to strip out the superfluous parts in your example program, I've rewritten it down to the following. Could you verify that the below program also crashes for you with a double-free? I tried to isolate specifically the scenario you described (error from OpenSSL + dropping multi handle without removing easy handles):
use curl::{
easy::{Easy, HttpVersion},
multi::Multi,
};
use std::time::Duration;
fn main() {
let multi = Multi::new();
let mut handles = Vec::new();
for _ in 0..2 {
let mut easy = Easy::new();
easy.url("https://self-signed.badssl.com").unwrap();
easy.http_version(HttpVersion::V2).unwrap();
handles.push(multi.add(easy).unwrap());
}
while multi.perform().unwrap() > 0 {
multi.wait(&mut [], Duration::from_secs(5)).unwrap();
}
drop(multi);
println!("Multi handle freed");
drop(handles);
println!("Easy handles freed");
}
So far I am not able to reproduce this on my local machine or inside a Centos 7.3.1611 Docker container. Are you also building OpenSSL and nghttp2 yourself? I see you are pointing to nonstandard paths for those. It would be helpful to see how you are building those.
It might also help to strip out the superfluous parts in your example program, I've rewritten it down to the following. Could you verify that the below program also crashes for you with a double-free? I tried to isolate specifically the scenario you described (error from OpenSSL + dropping multi handle without removing easy handles):
use curl::{ easy::{Easy, HttpVersion}, multi::Multi, }; use std::time::Duration; fn main() { let multi = Multi::new(); let mut handles = Vec::new(); for _ in 0..2 { let mut easy = Easy::new(); easy.url("https://self-signed.badssl.com").unwrap(); easy.http_version(HttpVersion::V2).unwrap(); handles.push(multi.add(easy).unwrap()); } while multi.perform().unwrap() > 0 { multi.wait(&mut [], Duration::from_secs(5)).unwrap(); } drop(multi); println!("Multi handle freed"); drop(handles); println!("Easy handles freed"); }
Sorry for my lack of rigor. Your version don't crash on my machine, so I do some modify and finally it crashed:
use curl::{
easy::{Easy, HttpVersion},
multi::Multi,
};
use std::time::Duration;
fn main() { let multi = Multi::new(); let mut handles = Vec::new();
for _ in 0..2 {
let mut easy = Easy::new();
easy.url("https://self-signed.badssl.com").unwrap();
easy.http_version(HttpVersion::V2).unwrap();
easy.pipewait(true).unwrap();
handles.push(multi.add(easy).unwrap());
}
let mut some_msg_result = None;
while some_msg_result.is_none() {
multi.perform().unwrap();
multi.messages(|msg| {
some_msg_result = msg.result();
});
multi.wait(&mut [], Duration::from_secs(5)).unwrap();
}
drop(handles);
println!("Easy handles freed");
drop(multi);
println!("Multi handle freed");
}
I also try to build it with mesalink backend, it crash too, but I copy the build result to other machine which has differece network env it won't crash, it may be network IO related.
Thanks, having a minimal repro will make it easier to identify the problem and apply the appropriate fix. Its interesting that dropping the easy handles first is part of the issue, since curl_easy_cleanup
calls curl_multi_remove_handle
internally anyway if needed.
Thanks, having a minimal repro will make it easier to identify the problem and apply the appropriate fix. Its interesting that dropping the easy handles first is part of the issue, since
curl_easy_cleanup
callscurl_multi_remove_handle
internally anyway if needed.
It seem there is "safe guard" when cleanup, all I know now is double free this pointer and it‘s not because of duplicate conn_free call. I will do more debug later.
Set a http proxy to easy handle will reproduce this preblem, on my machine it's setting by the $http_proxy
environment variable, I also tested this simple proxy by easy.proxy
call and it crashed too. @sagebind Can you try again?
@iiibui Thanks, that did the trick, I can consistently reproduce the double-free crash now as well. I'll work on stripping down the repro program further and hopefully identify the cause of why conn_free
in libcurl is being called multiple times.
I think this might be an upstream curl bug. Using git bisect I was able to identify that this starts happening after this commit, which definitely looks like it could be related to this sort of problem: https://github.com/curl/curl/commit/51c0ebcff2140c38ff389b4fcfb8216f5e9d198c. I will work on translating the repro into a C program so we can open a ticket upstream.
In the meantime, @iiibui could you verify my findings? If you build libcurl at commit 63813a0325adec659bdb6866c061208266b68797
and run your original example program, there should be no issue, but at commit 51c0ebcff2140c38ff389b4fcfb8216f5e9d198c
should cause double-free again like master does.
For reference, I am able to consistently reproduce a double free now with the following program:
use curl::{easy::Easy, multi::Multi};
use std::{
io::{copy, BufRead, BufReader, Write},
net::{SocketAddr, TcpListener, TcpStream},
thread,
};
fn main() {
let proxy_addr = spawn_http_proxy();
let multi = Multi::new();
let mut easy = Easy::new();
easy.url("https://self-signed.badssl.com").unwrap();
easy.proxy(&format!("http://{}", proxy_addr)).unwrap();
let easy = multi.add(easy).unwrap();
multi.perform().unwrap();
drop(easy);
println!("Easy handle freed");
drop(multi);
println!("Multi handle freed");
}
/// Spawn a simple HTTP proxy in a background thread for curl to talk to. Really
/// inefficient with threads but also very simple.
fn spawn_http_proxy() -> SocketAddr {
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let addr = listener.local_addr().unwrap();
thread::spawn(move || loop {
let (mut client, _) = listener.accept().unwrap();
thread::spawn(move || {
let mut reader = BufReader::new(client.try_clone().unwrap());
let mut request_header = String::new();
while !request_header.contains("\r\n\r\n") {
reader.read_line(&mut request_header).unwrap();
}
client.write_all(b"HTTP/1.1 200 OK\r\n\r\n").unwrap();
let upstream_addr = request_header.split(' ').nth(1).unwrap();
let mut upstream_reader = TcpStream::connect(upstream_addr).unwrap();
let mut upstream_writer = upstream_reader.try_clone().unwrap();
thread::spawn(move || {
let _ = copy(&mut reader, &mut upstream_writer);
});
let _ = copy(&mut upstream_reader, &mut client);
});
});
addr
}
Upstream issue opened: https://github.com/curl/curl/issues/7982
I think this might be an upstream curl bug. Using git bisect I was able to identify that this starts happening after this commit, which definitely looks like it could be related to this sort of problem: curl/curl@51c0ebc. I will work on translating the repro into a C program so we can open a ticket upstream.
In the meantime, @iiibui could you verify my findings? If you build libcurl at commit
63813a0325adec659bdb6866c061208266b68797
and run your original example program, there should be no issue, but at commit51c0ebcff2140c38ff389b4fcfb8216f5e9d198c
should cause double-free again like master does.For reference, I am able to consistently reproduce a double free now with the following program:
use curl::{easy::Easy, multi::Multi}; use std::{ io::{copy, BufRead, BufReader, Write}, net::{SocketAddr, TcpListener, TcpStream}, thread, }; fn main() { let proxy_addr = spawn_http_proxy(); let multi = Multi::new(); let mut easy = Easy::new(); easy.url("https://self-signed.badssl.com").unwrap(); easy.proxy(&format!("http://{}", proxy_addr)).unwrap(); let easy = multi.add(easy).unwrap(); multi.perform().unwrap(); drop(easy); println!("Easy handle freed"); drop(multi); println!("Multi handle freed"); } /// Spawn a simple HTTP proxy in a background thread for curl to talk to. Really /// inefficient with threads but also very simple. fn spawn_http_proxy() -> SocketAddr { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); thread::spawn(move || loop { let (mut client, _) = listener.accept().unwrap(); thread::spawn(move || { let mut reader = BufReader::new(client.try_clone().unwrap()); let mut request_header = String::new(); while !request_header.contains("\r\n\r\n") { reader.read_line(&mut request_header).unwrap(); } client.write_all(b"HTTP/1.1 200 OK\r\n\r\n").unwrap(); let upstream_addr = request_header.split(' ').nth(1).unwrap(); let mut upstream_reader = TcpStream::connect(upstream_addr).unwrap(); let mut upstream_writer = upstream_reader.try_clone().unwrap(); thread::spawn(move || { let _ = copy(&mut reader, &mut upstream_writer); }); let _ = copy(&mut upstream_reader, &mut client); }); }); addr }
Yes, with commit 63813a0325adec659bdb6866c061208266b68797
my original example program has no issue and commit 51c0ebcff2140c38ff389b4fcfb8216f5e9d198c
cause double-free.
The fix for this is now available in curl 0.4.41.
The fix for this is now available in curl 0.4.41.
Thanks. @ehuss I rebuild cargo 22ff7ac47c0e3a366637643c9cf38c61d649c10b which the rust-curl dep is aready updated to 0.4.41, I cannot reproduce the double-free crash on my machine now.
Problem
OS: CentOS Linux release 7.3.1611 (Core)
Use curl::multi::Multi to fetch more than one https url will cause the program to crash when some ssl error occurred(rel issue):
Rust code
The url https://127.0.0.1:433 in the code is a service implemented by the simple golang code:
server.crt is a self signed certificate generated by OpenSSL, you can replace with your version.
Cargo.toml
curl -V: