113 lines
4.2 KiB
Diff
113 lines
4.2 KiB
Diff
From 88b9a8ff19ac55c43b714fefc28d718fb4dcd611 Mon Sep 17 00:00:00 2001
|
|
From: Petr Vorel <pvorel@suse.cz>
|
|
Date: Fri, 9 May 2025 17:18:09 +0200
|
|
Subject: [PATCH 2/4] ping: Fix integer overflow in large -W value
|
|
|
|
918e824 changed probably as a side effect max -W (time to wait for a
|
|
response in sec) value from INT_MAX / 1000000 (i.e. 2147 s => ~ 35 min)
|
|
to INT_MAX / 1000 (i.e. 2147483 s => ~ 586 h). This allows int overflow
|
|
with -W > 2148 s (value which was not previously allowed):
|
|
|
|
$ export CFLAGS="-O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer"
|
|
$ meson setup ..
|
|
$ ninja && ./ping/ping -c1 -W 2148 8.8.8.8
|
|
../ping/ping_common.c:269:37: runtime error: signed integer overflow: 2148000 * 1000 cannot be represented in type 'int'
|
|
|
|
It could be fixed by simple casting global_rts->lingertime in:
|
|
waittime = (unsigned long)global_rts->lingertime * 1000;
|
|
|
|
But because max -W value is unreasonably large anyway fix the problem by:
|
|
1) storing lingertime as uint32_t (fixed 32bit unsigned int, requires C99)
|
|
instead of previous int (first contribution to
|
|
https://github.com/iputils/iputils/issues/410).
|
|
|
|
2) Converting lingertime to us during getopts parsing (ping since ever -
|
|
git era was converting lingertime during getopts only to ms, converting
|
|
to us was done for some reason later in __schedule_exit()).
|
|
|
|
New -W max value is now 71 min (over 1 hour) which should be enough
|
|
(-W also allows -W0 for an infinite timeout, see 3b43f90):
|
|
|
|
$ ./ping/ping -c1 -W 4294 8.8.8.8
|
|
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
|
|
64 bytes from 8.8.8.8: icmp_seq=1 ttl=116 time=13.4 ms
|
|
|
|
--- 8.8.8.8 ping statistics ---
|
|
1 packets transmitted, 1 received, 0% packet loss, time 0ms
|
|
rtt min/avg/max/mdev = 13.422/13.422/13.422/0.000 ms
|
|
|
|
Fixes: 918e824 ("ping: add support for sub-second timeouts")
|
|
Closes: https://github.com/iputils/iputils/pull/588
|
|
Reported-by: Mohamed Maatallah <hotelsmaatallahrecemail@gmail.com>
|
|
Reviewed-by: Mohamed Maatallah <hotelsmaatallahrecemail@gmail.com>
|
|
Tested-by: Mohamed Maatallah <hotelsmaatallahrecemail@gmail.com>
|
|
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
|
|
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
|
|
[ pvorel: backport of upstream f7d19893aed9188de758b6be940be01501b5315b to 20221126 ]
|
|
Signed-off-by: Petr Vorel <pvorel@suse.cz>
|
|
---
|
|
ping/ping.c | 6 +++---
|
|
ping/ping.h | 2 +-
|
|
ping/ping_common.c | 7 ++++---
|
|
3 files changed, 8 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/ping/ping.c b/ping/ping.c
|
|
index 02d358e..814ee71 100644
|
|
--- a/ping/ping.c
|
|
+++ b/ping/ping.c
|
|
@@ -543,10 +543,10 @@ main(int argc, char **argv)
|
|
double optval;
|
|
|
|
optval = ping_strtod(optarg, _("bad linger time"));
|
|
- if (isless(optval, 0) || isgreater(optval, (double)INT_MAX / 1000))
|
|
+ if (isless(optval, 0) || isgreater(optval, (double)UINT_MAX / 1000000))
|
|
error(2, 0, _("bad linger time: %s"), optarg);
|
|
- /* lingertime will be converted to usec later */
|
|
- rts.lingertime = (int)(optval * 1000);
|
|
+
|
|
+ rts.lingertime = (uint32_t)(optval * 1000000);
|
|
}
|
|
break;
|
|
default:
|
|
diff --git a/ping/ping.h b/ping/ping.h
|
|
index 46cfbe7..f667c62 100644
|
|
--- a/ping/ping.h
|
|
+++ b/ping/ping.h
|
|
@@ -163,7 +163,7 @@ struct ping_rts {
|
|
int interval; /* interval between packets (msec) */
|
|
int preload;
|
|
int deadline; /* time to die */
|
|
- int lingertime;
|
|
+ uint32_t lingertime;
|
|
struct timespec start_time, cur_time;
|
|
volatile int exiting;
|
|
volatile int status_snapshot;
|
|
diff --git a/ping/ping_common.c b/ping/ping_common.c
|
|
index 0d0da76..0fbb825 100644
|
|
--- a/ping/ping_common.c
|
|
+++ b/ping/ping_common.c
|
|
@@ -264,8 +264,9 @@ int __schedule_exit(int next)
|
|
waittime = 2 * global_rts->tmax;
|
|
if (waittime < 1000 * (unsigned long)global_rts->interval)
|
|
waittime = 1000 * global_rts->interval;
|
|
- } else
|
|
- waittime = global_rts->lingertime * 1000;
|
|
+ } else {
|
|
+ waittime = global_rts->lingertime;
|
|
+ }
|
|
|
|
if (next < 0 || (unsigned long)next < waittime / 1000)
|
|
next = waittime / 1000;
|
|
@@ -387,7 +388,7 @@ resend:
|
|
if (nores_interval > 500)
|
|
nores_interval = 500;
|
|
oom_count++;
|
|
- if (oom_count * nores_interval < rts->lingertime)
|
|
+ if ((uint32_t)(oom_count * nores_interval) < rts->lingertime)
|
|
return nores_interval;
|
|
i = 0;
|
|
/* Fall to hard error. It is to avoid complete deadlock
|
|
--
|
|
2.49.0
|
|
|