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

segmentation fault using a easy handle after CURLE_WRITE_ERROR #9873

Closed
Edrusb opened this issue Nov 8, 2022 · 8 comments
Closed

segmentation fault using a easy handle after CURLE_WRITE_ERROR #9873

Edrusb opened this issue Nov 8, 2022 · 8 comments
Labels
crash not-a-curl-bug This is not a bug in curl

Comments

@Edrusb
Copy link

Edrusb commented Nov 8, 2022

I did this

the application I maintain and that relies on libcurl sometime needs to stop reading file content from ftp/sftp depending on the data read so far.

For years it was implemented by CURLOPT_WRITEFUNCTION + CURLOPT_WRITEDATA and the pointed to callback returned 0 instead of the size*nmemb value when reading needed to stop (sftp and ftp protocols).

Today, if doing that way still works as expected, (curl_easy_perform() returns the expected CURLE_WRITE_ERROR code) any subsequent further action on the handle triggers a segmentation fault with the following stack (if that can help):

(gdb) n
[New Thread 0x7ffff7292700 (LWP 25812)]

Thread 5 "dar" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7292700 (LWP 25812)]
0x00007ffff75bb298 in ?? () from /lib/x86_64-linux-gnu/libnss_files.so.2
(gdb) bt
#0  0x00007ffff75bb298 in ?? () from /lib/x86_64-linux-gnu/libnss_files.so.2
#1  0x00007ffff75bc544 in _nss_files_gethostbyname4_r () from /lib/x86_64-linux-gnu/libnss_files.so.2
#2  0x0000000000c05a76 in gaih_inet.constprop ()
#3  0x0000000000c068f5 in getaddrinfo ()
#4  0x000000000067d3c4 in Curl_getaddrinfo_ex ()
#5  0x0000000000675e83 in getaddrinfo_thread ()
#6  0x000000000067decb in curl_thread_create_thunk ()
#7  0x00000000008d9b97 in start_thread (arg=<optimized out>) at pthread_create.c:477
#8  0x0000000000c0cb5f in clone ()
(gdb)

I tried to curl_easy_reset() first and reapplying all option (curl_easy_setopt()) on this handle, this just leads to the same fate...
I tried also after a curl_easy_cleanup() + curl_easy_init() ... same fate...

I expected the following

if I'm doing something wrong which might still be possible, an error message would be welcome ;)

curl/libcurl version

curl 7.74.0 (x86_64-pc-linux-gnu) libcurl/7.85.0 OpenSSL/1.1.1n zlib/1.2.11 zstd/1.4.8 libssh2/1.9.0
Release-Date: 2020-12-09
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets zstd
WARNING: curl and libcurl versions do not match. Functionality may be affected.

note that only libcurl is used (compiled by myself to have a statically linkable library, something my distro does not provide), where from the curl/liburl difference in the previous output

operating system

Linux Devuan
lsb_release -a

No LSB modules are available.
Distributor ID: Devuan
Description:    Devuan GNU/Linux 4 (chimaera)
Release:        4
Codename:       chimaera

uname -a

Linux terre-neuve 5.10.0-16-amd64 #1 SMP Debian 5.10.127-2 (2022-07-23) x86_64 GNU/Linux

@bagder bagder added the crash label Nov 8, 2022
@bagder
Copy link
Member

bagder commented Nov 8, 2022

That looks like a crash inside getaddrinfo that curl is not to blame for.

jay added a commit to jay/curl that referenced this issue Nov 8, 2022
Prior to this change if the user wanted to signal an error from their
write callbacks they would have to use logic to return a value different
from the number of bytes (nmemb) passed to the callback. Also, the
inclination of some users has been to just return 0 to signal error,
which is incorrect as that may be the number of bytes passed to the
callback.

To remedy this the user can now return CURL_WRITEFUNC_ERROR instead.

Ref: curl#9873

Closes #xxxx
@jay
Copy link
Member

jay commented Nov 8, 2022

and the pointed to callback returned 0 instead of the size*nmemb value when reading needed to stop (sftp and ftp protocols).

To be clear 0 only signals an error if nmemb is not 0, because then the amount of bytes handled is different from what was passed in. You would have to use logic to determine a proper failure code (example).

Anyway, so far there's nothing here to suggest the issue you reported is a libcurl issue. Can you give us a minimal self contained example that can be used to reproduce?

@Edrusb
Copy link
Author

Edrusb commented Nov 9, 2022

Yes, of course, if the provided nmemb is null, returning 0 from the callback will not trigger curl_easy_perform() to stop with CURLE_WRITE_ERROR (but unless the callback is constantly invoked with nmemb set to zero, the requested interruption of libcurl would occur a next time. But OK, this can be optimized on my side).

I will try to extract the calls done to libcurl from the C++ classes into a simpler C code, I just need a few days to find the time to do that.

@jay jay added the needs-info label Nov 10, 2022
@bagder
Copy link
Member

bagder commented Nov 10, 2022

Potentially this bug?

jay added a commit that referenced this issue Nov 10, 2022
Prior to this change if the user wanted to signal an error from their
write callbacks they would have to use logic to return a value different
from the number of bytes (nmemb) passed to the callback. Also, the
inclination of some users has been to just return 0 to signal error,
which is incorrect as that may be the number of bytes passed to the
callback.

To remedy this the user can now return CURL_WRITEFUNC_ERROR instead.

Ref: #9873

Closes #9874
@Edrusb
Copy link
Author

Edrusb commented Nov 10, 2022

Well, that does not match. I have the problem with both statically and dynamically linked binaries.

But during these testings, I found (by mistake) that the segfault expressed only when some options were set (CURLOPT_USERPWD attracted my attention) I still have to characterize more precisely which option is significant and is not eventually improperly set on my side. I will get back to you with more precised scenario/info.

But can you confirm that I properly understand the documentation when it tells "The application does not have to keep the string around after setting this option." : I understand that if the string provided to curl_easy_setopt(CURLOPT_USERPWD,...) is released before curl_easy_perform() is invoked, this should not cause any harm, correct?

@jay
Copy link
Member

jay commented Nov 11, 2022

if the string provided to curl_easy_setopt(CURLOPT_USERPWD,...) is released before curl_easy_perform() is invoked, this should not cause any harm, correct?

Yes, that is correct.

@Edrusb
Copy link
Author

Edrusb commented Nov 11, 2022

after reviewing my testing environment, I realized that my dynamically linked binaries were upx-compressed (they are built this way by default), which lead the 'file' command to report them as statically linked (not sure how it impacts on the bug concerning getaddrinfo() you mentioned). Anyway, disabling upx compression I get the segfault only with real statically linked binaries, no problem with dynamically linked one.

What troubled me is that calling the statically linked binary a given way led the segfault to occur, but calling it another way did not. I added some printf() calls before the invocations to curl_* routines and realized that both ways ended with the almost same sequence of call to the libcurl API!

curl_global_init(CURL_GLOBAL_ALL) -> 0
curl_version_info(CURLVERSION_FOURTH) -> 15553152
curl_easy_init() -> 42770704
curl_easy_setopt(42770704, 48, 1) -> 0
curl_easy_setopt(42770704, 10001, 42700832) -> 0
curl_easy_setopt(42770704, 10002, ftp://jupiter/) -> 0
curl_easy_setopt(42770704, 10005, dar-check:totototo) -> 0
curl_easy_setopt(42770704, 20011, 5718504) -> 0
curl_easy_perform(42770704) ...
Segmentation fault

or

curl_global_init(CURL_GLOBAL_ALL) -> 0
curl_version_info(CURLVERSION_FOURTH) -> 15553152
curl_easy_init() -> 16083440
curl_easy_setopt(16083440, 48, 1) -> 0
curl_easy_setopt(16083440, 10001, 16068640) -> 0
curl_easy_setopt(16083440, 10002, ftp://jupiter/) -> 0
curl_easy_setopt(16083440, 10005, dar-check:totototo) -> 0
curl_easy_setopt(16083440, 20011, 5718504) -> 0
curl_easy_perform(16083440) ...
curl_easy_perform(16083440) -> 0
curl_easy_setopt(16083440, 44, 1) -> 0
curl_easy_setopt(16083440, 48, 0) -> 0
curl_easy_setopt(16083440, 10001, 16097248) -> 0
curl_easy_setopt(16083440, 10002, ftp://jupiter/tutu.1.dar) -> 0
curl_easy_setopt(16083440, 20011, 6395876) -> 0
curl_easy_perform(16083440) ...
curl_easy_perform(16083440) -> 0
curl_easy_getinfo(16083440, 3145743, 1221545128); -> 0
curl_easy_setopt(16083440, 44, 0) -> 0
curl_easy_setopt(16083440, 20011, 6394526) -> 0
curl_easy_setopt(16083440, 30116, 0) -> 0
curl_easy_perform(16083440) ...
curl_easy_perform(16083440) -> 23
curl_easy_setopt(16083440, 30116, 39648013) -> 0
curl_easy_perform(16083440) ...
curl_easy_perform(16083440) -> 0
curl_easy_setopt(16083440, 30116, 39144677) -> 0
curl_easy_perform(16083440) ...
curl_easy_perform(16083440) -> 0
curl_easy_cleanup(16083440);
curl_global_cleanup();

in the first sequence ending with a segfault, only a somehow simple routine is called in addition, to display a message to the user, but this message may be translated by mean of gettext(). When the LANG variable is unset no segfault occurs, when it is set to my locale the segfault occurs.... I've checked both native formatting string and its translated version, they have the same % format in type and values. Removing the gettext() call avoids the segfault while the LANG is still set to my locale... I don't understand for now why it only concerns statically linked version... but that's another story.

In conclusion, IMHO, the problem is not libcurl related at all, I understand there is some memory corruption done around the gettext() call and translated string, which lead to the segfault some time after in _nss_files_gethostbyname4_r() which is called (indirectly) from libcurl.

thank you for your support and this very useful library :)

@jay jay added not-a-curl-bug This is not a bug in curl and removed needs-info labels Nov 11, 2022
@jay
Copy link
Member

jay commented Nov 11, 2022

Thanks for following up. You could try valgrind or address sanitizer to further narrow it down. I don't think there's enough information here to definitively identify the cause, but I also think it's highly unlikely this is a libcurl issue so I'm closing.

@jay jay closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash not-a-curl-bug This is not a bug in curl
Development

No branches or pull requests

3 participants