Subject: ping: fix main loop over multiple addrinfo results
From: Benjamin Poirier <[email-masked]>
Date: Thu, 26 Dec 2019 10:44:03 +0900

Despite what the log of commit f68eec0eafad ("ping: perform dual-stack ping
by default") says, main() was not designed to loop over multiple addresses
returned by getaddrinfo().  This is apparent because until commit
db11bc96a68c ("ping: make command to return from main()"), ping{4,6}_run()
never returned (they always exited).  After commit db11bc96a68c, we
encounter unexpected situations if getaddrinfo returns multiple results and
ping{4,6}_run() return != 0.

For example (notice echo reply is not received):

    root@vsid:/src/iputils# ./builddir/ping/ping -w1 google.com
    PING google.com(nrt12s22-in-x0e.1e100.net (2404:6800:4004:80c::200e)) 56 data bytes

    --- google.com ping statistics ---
    1 packets transmitted, 0 received, 100% packet loss, time 0ms

    PING  (216.58.197.142) 56(84) bytes of data.

    ---  ping statistics ---
    1 packets transmitted, 0 received, 100% packet loss, time -1002ms

    root@vsid:/src/iputils#

Establish the following convention:

* return value >= 0 -> exit with this code (same behavior as before commit
  db11bc96a68c)

* return value < 0 -> go on to next addrinfo result

The second case will be used in the following patch.

Fixes: db11bc96a68c ("ping: make command to return from main()")
Signed-off-by: Benjamin Poirier <[email-masked]>
Origin: https://github.com/iputils/iputils/commit/c249e48bb865e731896b7d8ceaf4bca7d28768b6
Bug-Debian: https://bugs.debian.org/947921
See-also: https://github.com/iputils/iputils/pull/244
---
 ping.c         | 6 +++++-
 ping6_common.c | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ping.c b/ping.c
index 34653be..ae687b4 100644
--- a/ping.c
+++ b/ping.c
@@ -495,8 +495,11 @@ main(int argc, char **argv)
 			error(2, 0, _("unknown protocol family: %d"), ai->ai_family);
 		}
 
-		if (ret_val == 0)
+		if (ret_val >= 0)
 			break;
+		/* ret_val < 0 means to go on to next addrinfo result, there
+		 * better be one. */
+		assert(ai->ai_next);
 	}
 
 	freeaddrinfo(result);
@@ -504,6 +507,7 @@ main(int argc, char **argv)
 	return ret_val;
 }
 
+/* return >= 0: exit with this code, < 0: go on to next addrinfo result */
 int ping4_run(int argc, char **argv, struct addrinfo *ai, socket_st *sock)
 {
 	static const struct addrinfo hints = {
diff --git a/ping6_common.c b/ping6_common.c
index 6cc5404..731dc6d 100644
--- a/ping6_common.c
+++ b/ping6_common.c
@@ -551,6 +551,7 @@ int niquery_option_handler(const char *opt_arg)
 	return ret;
 }
 
+/* return >= 0: exit with this code, < 0: go on to next addrinfo result */
 int ping6_run(int argc, char **argv, struct addrinfo *ai, struct socket_st *sock)
 {
 	static const struct addrinfo hints = {
-- 
2.25.0.rc2