There are cases for ping(8)'s "-i wait" option that we don't handle
correctly.
Negative values smaller than -1:
$ doas ping -i -0.1 localhost
[no error]
Positive values less than one microsecond:
$ doas ping -i 0.0000001 localhost
[no error]
Large positive values that actually fit into a timeval:
$ ping -i $(bc -e '2^63 - 1' -e quit) localhost
ping: illegal timing interval 9223372036854775807
The first two cases are bugs in the input checking. The latter case
is a limitation of IEEE doubles: with only 52 bits of significand you
can't represent the full range of a timeval on a platform with 64-bit
time_t.
This patch addresses the first two cases with better error checking.
It also tries to handle the latter case a bit better by using IEEE
quads, i.e. long doubles. With 64 bits of significand you can cover
our time_t and the above case works. It isn't ~perfect~, but it's as
close as we can get to perfect without parsing the number by hand as
we do in sleep(1) (cf. bin/sleep/sleep.c).
With this patch those cases all work as expected:
$ doas ping -i -0.1 localhost
ping: interval is invalid: -0.1
$ doas ping -i 0.0000001 localhost
ping: interval is too small: 0.0000001
$ ping -i $(bc -e '2^63' -e quit) localhost
ping: interval is too large: 9223372036854775808
$ ping -i $(bc -e '2^63 - 1' -e quit) localhost
PING localhost.local (23.195.69.106): 56 data bytes
64 bytes from 23.195.69.106: icmp_seq=0 ttl=54 time=18.001 ms
[stalls forever]
--
While we're modifying this code, I think "-i interval" is better than
"-i wait". The "i for interval" mnemonic is particularly nice. The
current "wait" argument name sort-of implies a relationship with the
"-w maxwait" option, which is not the case. We're also already using
"timing interval" in these error messages here. I've tweaked the
error messages to look more like the usual strtonum(3) error messages.
ok?
Index: ping.c
===================================================================
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.237
diff -u -p -r1.237 ping.c
--- ping.c 20 Jul 2019 00:49:54 -0000 1.237
+++ ping.c 20 Jul 2019 12:46:46 -0000
@@ -258,7 +258,7 @@ main(int argc, char *argv[])
char *e, *target, hbuf[NI_MAXHOST], *source = NULL;
char rspace[3 + 4 * NROUTES + 1]; /* record route space */
const char *errstr;
- double intval;
+ long double dec, frac, seconds;
uid_t ouid, uid;
gid_t gid;
u_int rtableid = 0;
@@ -332,17 +332,21 @@ main(int argc, char *argv[])
case 'S': /* deprecated */
source = optarg;
break;
- case 'i': /* wait between sending packets */
- intval = strtod(optarg, &e);
- if (*optarg == '\0' || *e != '\0')
- errx(1, "illegal timing interval %s", optarg);
- if (intval < 1 && ouid)
- errx(1, "only root may use interval < 1s");
- interval.tv_sec = (time_t)intval;
- interval.tv_usec =
- (long)((intval - interval.tv_sec) * 1000000);
- if (interval.tv_sec < 0)
- errx(1, "illegal timing interval %s", optarg);
+ case 'i': /* interval between packets */
+ seconds = strtold(optarg, &e);
+ if (*optarg == '\0' || *e != '\0' || seconds < 0.0)
+ errx(1, "interval is invalid: %s", optarg);
+ frac = modfl(seconds, &dec);
+ if (dec > LLONG_MAX)
+ errx(1, "interval is too large: %s", optarg);
+ interval.tv_sec = dec;
+ interval.tv_usec = frac * 1000000.0;
+ if (!timerisset(&interval))
+ errx(1, "interval is too small: %s", optarg);
+ if (interval.tv_sec < 1 && ouid != 0) {
+ errx(1, "only root may use an interval smaller"
+ "than one second");
+ }
options |= F_INTERVAL;
break;
case 'L':
@@ -2176,13 +2180,13 @@ usage(void)
if (v6flag) {
fprintf(stderr,
"usage: ping6 [-DdEefHLmnqv] [-c count] [-h hoplimit] "
- "[-I sourceaddr]\n\t[-i wait] [-l preload] [-p pattern] "
- "[-s packetsize] [-T toskeyword]\n\t"
- "[-V rtable] [-w maxwait] host\n");
+ "[-I sourceaddr]\n\t[-i interval] [-l preload] "
+ "[-p pattern] [-s packetsize] [-T toskeyword]\n"
+ "\t[-V rtable] [-w maxwait] host\n");
} else {
fprintf(stderr,
- "usage: ping [-DdEefHLnqRv] [-c count] [-I ifaddr]"
- " [-i wait]\n\t[-l preload] [-p pattern] [-s packetsize]"
+ "usage: ping [-DdEefHLnqRv] [-c count] [-I ifaddr] "
+ "[-i interval]\n\t[-l preload] [-p pattern] [-s packetsize]"
#ifndef SMALL
" [-T toskeyword]"
#endif /* SMALL */
Index: ping.8
===================================================================
RCS file: /cvs/src/sbin/ping/ping.8,v
retrieving revision 1.61
diff -u -p -r1.61 ping.8
--- ping.8 10 Nov 2018 23:44:53 -0000 1.61
+++ ping.8 20 Jul 2019 12:46:46 -0000
@@ -69,7 +69,7 @@
.Op Fl DdEefHLnqRv
.Op Fl c Ar count
.Op Fl I Ar ifaddr
-.Op Fl i Ar wait
+.Op Fl i Ar interval
.Op Fl l Ar preload
.Op Fl p Ar pattern
.Op Fl s Ar packetsize
@@ -83,7 +83,7 @@
.Op Fl c Ar count
.Op Fl h Ar hoplimit
.Op Fl I Ar sourceaddr
-.Op Fl i Ar wait
+.Op Fl i Ar interval
.Op Fl l Ar preload
.Op Fl p Ar pattern
.Op Fl s Ar packetsize
@@ -160,13 +160,15 @@ Set the hoplimit.
Specify the interface address to transmit from
on machines with multiple interfaces.
For unicast and multicast pings.
-.It Fl i Ar wait
-Wait
-.Ar wait
-seconds between sending each packet.
-The default is to wait for one second between each packet.
-The wait time may be fractional, but only the superuser may specify
-a value less than one second.
+.It Fl i Ar interval
+Wait at least
+.Ar interval
+seconds between each outgoing packet.
+The default is one second.
+The
+.Ar interval
+may contain a fractional portion.
+Only the superuser may specify a value less than one second.
This option is incompatible with the
.Fl f
option.
