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

illegal instruction in progress_meter #9082

Closed
geeknik opened this issue Jul 1, 2022 · 3 comments
Closed

illegal instruction in progress_meter #9082

geeknik opened this issue Jul 1, 2022 · 3 comments
Labels

Comments

@geeknik
Copy link

@geeknik geeknik commented Jul 1, 2022

I did this

compiled curl with clang-12 + undefined behavior sanitizer
I then ran ./curl -K test0000 file:///dev/null

test0000 base64:
LU9adwAAUmNoIDg0

I expected the following

No crash.

But instead this happened

Program received signal SIGILL, Illegal instruction.
0x0000000000273323 in progress_meter (global=0x7fffffffe198, start=<optimized out>, final=false) at tool_progress.c:271
271           dls = (curl_off_t)((double)dl / ((double)deltams/1000.0));
(gdb) bt
#0  0x0000000000273323 in progress_meter (global=0x7fffffffe198, start=<optimized out>, final=false) at tool_progress.c:271
#1  0x000000000025abc5 in parallel_transfers (global=0x7fffffffe198, share=0x5a4060) at tool_operate.c:2268
#2  run_all_transfers (global=0x7fffffffe198, share=0x5a4060, result=CURLE_OK) at tool_operate.c:2571
#3  operate (global=0x7fffffffe198, argc=<optimized out>, argv=0x0) at tool_operate.c:2689
#4  main (argc=<optimized out>, argv=0x0) at tool_main.c:276

curl/libcurl version

./curl --version
curl 7.84.1-DEV (x86_64-pc-linux-gnu) libcurl/7.84.1-DEV zlib/1.2.11 libidn2/2.2.0
Release-Date: [unreleased]
Protocols: dict file ftp gopher http imap mqtt pop3 rtsp smtp telnet tftp
Features: alt-svc AsynchDNS IDN IPv6 Largefile libz threadsafe UnixSockets
root@dtf:~/curl/src/tmp/out/default/crashes#

Git commit a8e02881ec9417706610443bcfee6e1104bb44c6

operating system

Linux 5.4.0-121-generic #137-Ubuntu SMP Wed Jun 15 13:33:07 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

@bagder bagder added the crash label Jul 1, 2022
@bagder
Copy link
Member

@bagder bagder commented Jul 1, 2022

Could that be a division by zero that appears like that? The divisions on line 271 and 272 seem a bit too "unprotected"...

@bagder
Copy link
Member

@bagder bagder commented Jul 1, 2022

If so, this could be a fix:

diff --git a/src/tool_progress.c b/src/tool_progress.c
index da5317b92..46185c0d3 100644
--- a/src/tool_progress.c
+++ b/src/tool_progress.c
@@ -266,10 +266,12 @@ bool progress_meter(struct GlobalConfig *global,
         /* since the beginning */
         deltams = tvdiff(now, *start);
         dl = all_dlnow;
         ul = all_ulnow;
       }
+      if(!deltams) /* no division by zero please */
+        deltams++;
       dls = (curl_off_t)((double)dl / ((double)deltams/1000.0));
       uls = (curl_off_t)((double)ul / ((double)deltams/1000.0));
       speed = dls > uls ? dls : uls;
     }

@geeknik
Copy link
Author

@geeknik geeknik commented Jul 1, 2022

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants