From c792b5acc3c34b62dd5cbe1026e3c178c051bc5e Mon Sep 17 00:00:00 2001 From: Yudong Cui Date: Fri, 5 Jul 2024 14:07:46 +0800 Subject: [PATCH] backport upstream code and fix bug --- ...random-value-for-the-identifier-fiel.patch | 155 ++++++++++++++++++ ...iff-Set-ppoll-timeout-minimum-to-1ms.patch | 30 ++++ ...port-ping-Fix-overflow-on-negative-i.patch | 30 ++++ ...ng-Fix-the-errno-handling-for-strtod.patch | 63 +++++++ ...rval-correctly-in-the-second-after-b.patch | 36 ++++ ...port-ping.h-Remove-duplicate-include.patch | 26 +++ ...port-for-DSCP-Traffic-Class-option-Q.patch | 54 ++++++ ...th-Don-t-assume-tv_sec-0-means-unset.patch | 32 ++++ iputils.spec | 24 ++- 9 files changed, 449 insertions(+), 1 deletion(-) create mode 100644 backport-Revert-ping-use-random-value-for-the-identifier-fiel.patch create mode 100644 backport-clockdiff-Set-ppoll-timeout-minimum-to-1ms.patch create mode 100644 backport-ping-Fix-overflow-on-negative-i.patch create mode 100644 backport-ping-Fix-the-errno-handling-for-strtod.patch create mode 100644 backport-ping-Handle-interval-correctly-in-the-second-after-b.patch create mode 100644 backport-ping.h-Remove-duplicate-include.patch create mode 100644 backport-ping6-Fix-support-for-DSCP-Traffic-Class-option-Q.patch create mode 100644 backport-tracepath-Don-t-assume-tv_sec-0-means-unset.patch diff --git a/backport-Revert-ping-use-random-value-for-the-identifier-fiel.patch b/backport-Revert-ping-use-random-value-for-the-identifier-fiel.patch new file mode 100644 index 0000000..77e2c6e --- /dev/null +++ b/backport-Revert-ping-use-random-value-for-the-identifier-fiel.patch @@ -0,0 +1,155 @@ +From d466aabcadcc2d7fd1f132ea3f580ad102773cf9 Mon Sep 17 00:00:00 2001 +From: Petr Vorel +Date: Wed, 6 Dec 2023 15:42:16 +0100 +Subject: [PATCH] Revert "ping: use random value for the identifier field" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This reverts commit 5026c2221a15bf13e601eade015c971bf07a27e9. + +Unlike TCP and UDP, which use port to uniquely identify the socket to +deliver data, ICMP use identifier field (ID) to identify the socket. + +Therefore if on the same machine, at the same time, two ping processes +use the same ID, echo reply can be delivered to the wrong socket. + +This is known problem due 16 bit ID field (65535). We used to use PID +to get unique number. The default value of /proc/sys/kernel/pid_max is +32768 (half). + +The problem is not new, but it was hidden until 5f6bec5 ("ping: Print +reply with wrong source with warning"). 5026c22 changed it to use our +random implementation to increase security. But that actually increases +the collisions on systems that use ping heavily: e.g. ping run with +Nagios via Debian specific check-host-alive Nagios plugin: + + $ ping -n -v -D -W 1 -i 1 -c 5 -M 'do' -s 56 -O "$Host") + +(75-100 ping instances in the reported issue.) + +Because we consider warning from 5f6bec5 useful and not consider leaking +PID information as a real security issue, we revert 5026c22. getpid() is +used in other ping implementations: + +* fping +https://github.com/schweikert/fping/blob/develop/src/fping.c#L496 + +* busybox +https://git.busybox.net/busybox/tree/networking/ping.c#n376 + +* FreeBSD +https://cgit.freebsd.org/src/tree/sbin/ping/ping.c#n632 + +* inetutils +https://git.savannah.gnu.org/cgit/inetutils.git/tree/ping/ping.c#n286 + +* Apple +https://opensource.apple.com/source/network_cmds/network_cmds-433/ping.tproj/ping.c.auto.html + +In case leaking PID *is* a real problem, we could solve this with +comparing the ICMP optional data. We could add 128 bit random value to +check. But we already use struct timeval if packet size is big enough +for it (>= 16 bits), therefore we could use it for comparing for most of +the packet sizes (the default is 56 bits). + +Fixes: https://github.com/iputils/iputils/issues/489 +Closes: https://github.com/iputils/iputils/pull/503 +Reported-by: Miloslav Hůla +Suggested-by: Cyril Hrubis +Acked-by: Johannes Segitz jsegitz@suse.de +Acked-by: Cyril Hrubis +Signed-off-by: Petr Vorel +--- + ping/node_info.c | 1 + + ping/ping.c | 4 +--- + ping/ping.h | 2 +- + ping/ping6_common.c | 2 +- + ping/ping_common.c | 4 ++-- + 5 files changed, 6 insertions(+), 7 deletions(-) + +diff --git a/ping/node_info.c b/ping/node_info.c +index 10a7681..ce392a2 100644 +--- a/ping/node_info.c ++++ b/ping/node_info.c +@@ -91,6 +91,7 @@ int niquery_is_enabled(struct ping_ni *ni) + void niquery_init_nonce(struct ping_ni *ni) + { + #if PING6_NONCE_MEMORY ++ iputils_srand(); + ni->nonce_ptr = calloc(NI_NONCE_SIZE, MAX_DUP_CHK); + if (!ni->nonce_ptr) + error(2, errno, "calloc"); +diff --git a/ping/ping.c b/ping/ping.c +index f470710..0ff5a48 100644 +--- a/ping/ping.c ++++ b/ping/ping.c +@@ -569,8 +569,6 @@ main(int argc, char **argv) + if (!argc) + error(1, EDESTADDRREQ, "usage error"); + +- iputils_srand(); +- + target = argv[argc - 1]; + + rts.outpack = malloc(rts.datalen + 28); +@@ -1527,7 +1525,7 @@ in_cksum(const unsigned short *addr, int len, unsigned short csum) + /* + * pinger -- + * Compose and transmit an ICMP ECHO REQUEST packet. The IP packet +- * will be added on by the kernel. The ID field is a random number, ++ * will be added on by the kernel. The ID field is our UNIX process ID, + * and the sequence number is an ascending integer. The first several bytes + * of the data portion are used to hold a UNIX "timeval" struct in VAX + * byte-order, to compute the round-trip time. +diff --git a/ping/ping.h b/ping/ping.h +index 04b2ccf..7799395 100644 +--- a/ping/ping.h ++++ b/ping/ping.h +@@ -159,7 +159,7 @@ struct ping_rts { + size_t datalen; + char *hostname; + uid_t uid; +- int ident; /* random id to identify our packets */ ++ int ident; /* process id to identify our packets */ + + int sndbuf; + int ttl; +diff --git a/ping/ping6_common.c b/ping/ping6_common.c +index 7b2bf15..5e78f85 100644 +--- a/ping/ping6_common.c ++++ b/ping/ping6_common.c +@@ -583,7 +583,7 @@ out: + /* + * pinger -- + * Compose and transmit an ICMP ECHO REQUEST packet. The IP packet +- * will be added on by the kernel. The ID field is a random number, ++ * will be added on by the kernel. The ID field is our UNIX process ID, + * and the sequence number is an ascending integer. The first several bytes + * of the data portion are used to hold a UNIX "timeval" struct in VAX + * byte-order, to compute the round-trip time. +diff --git a/ping/ping_common.c b/ping/ping_common.c +index ed4fee8..6eb1aa4 100644 +--- a/ping/ping_common.c ++++ b/ping/ping_common.c +@@ -303,7 +303,7 @@ void print_timestamp(struct ping_rts *rts) + /* + * pinger -- + * Compose and transmit an ICMP ECHO REQUEST packet. The IP packet +- * will be added on by the kernel. The ID field is a random number, ++ * will be added on by the kernel. The ID field is our UNIX process ID, + * and the sequence number is an ascending integer. The first several bytes + * of the data portion are used to hold a UNIX "timeval" struct in VAX + * byte-order, to compute the round-trip time. +@@ -536,7 +536,7 @@ void setup(struct ping_rts *rts, socket_st *sock) + } + + if (sock->socktype == SOCK_RAW) +- rts->ident = rand() & 0xFFFF; ++ rts->ident = htons(getpid() & 0xFFFF); + + set_signal(SIGINT, sigexit); + set_signal(SIGALRM, sigexit); +-- +2.33.0 + diff --git a/backport-clockdiff-Set-ppoll-timeout-minimum-to-1ms.patch b/backport-clockdiff-Set-ppoll-timeout-minimum-to-1ms.patch new file mode 100644 index 0000000..e635560 --- /dev/null +++ b/backport-clockdiff-Set-ppoll-timeout-minimum-to-1ms.patch @@ -0,0 +1,30 @@ +From 471942dee341e5aae2a277ecd85c05e671752880 Mon Sep 17 00:00:00 2001 +From: caibingcheng +Date: Mon, 24 Apr 2023 20:45:28 +0800 +Subject: [PATCH] clockdiff: Set ppoll timeout minimum to 1ms + +Fixes: https://github.com/iputils/iputils/issues/326 +Closes: https://github.com/iputils/iputils/pull/459 +Reviewed-by: Petr Vorel +Reviewed-by: Noah Meyerhans +Signed-off-by: caibingcheng +--- + clockdiff.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/clockdiff.c b/clockdiff.c +index 5e639ab..ccb5b5b 100644 +--- a/clockdiff.c ++++ b/clockdiff.c +@@ -210,7 +210,7 @@ static int measure_inner_loop(struct run_state *ctl, struct measure_vars *mv) + struct pollfd p = { .fd = ctl->sock_raw, .events = POLLIN | POLLHUP }; + + { +- long tmo = ctl->rtt + ctl->rtt_sigma; ++ long tmo = MAX(ctl->rtt + ctl->rtt_sigma, 1); + + mv->tout.tv_sec = tmo / 1000; + mv->tout.tv_nsec = (tmo - (tmo / 1000) * 1000) * 1000000; +-- +2.33.0 + diff --git a/backport-ping-Fix-overflow-on-negative-i.patch b/backport-ping-Fix-overflow-on-negative-i.patch new file mode 100644 index 0000000..5dd33c6 --- /dev/null +++ b/backport-ping-Fix-overflow-on-negative-i.patch @@ -0,0 +1,30 @@ +From 2a63b94313352ed62f47adbc1a0d33bfea7ca188 Mon Sep 17 00:00:00 2001 +From: Petr Vorel +Date: Wed, 17 May 2023 00:57:24 +0200 +Subject: [PATCH] ping: Fix overflow on negative -i + +Restore back check for -i <= 0. + +Fixes: 918e824 ("ping: add support for sub-second timeouts") +Closes: https://github.com/iputils/iputils/issues/465 +Signed-off-by: Petr Vorel +--- + ping/ping.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ping/ping.c b/ping/ping.c +index 8f44203..27c72ca 100644 +--- a/ping/ping.c ++++ b/ping/ping.c +@@ -436,7 +436,7 @@ main(int argc, char **argv) + double optval; + + optval = ping_strtod(optarg, _("bad timing interval")); +- if (isgreater(optval, (double)INT_MAX / 1000)) ++ if (islessequal(optval, 0) || isgreater(optval, (double)INT_MAX / 1000)) + error(2, 0, _("bad timing interval: %s"), optarg); + rts.interval = (int)(optval * 1000); + rts.opt_interval = 1; +-- +2.33.0 + diff --git a/backport-ping-Fix-the-errno-handling-for-strtod.patch b/backport-ping-Fix-the-errno-handling-for-strtod.patch new file mode 100644 index 0000000..ac3b0a1 --- /dev/null +++ b/backport-ping-Fix-the-errno-handling-for-strtod.patch @@ -0,0 +1,63 @@ +From 33e78be2e60ed9ac918dec13271d1bd9dce6e94e Mon Sep 17 00:00:00 2001 +From: Jacek Tomasiak +Date: Mon, 6 Feb 2023 13:39:44 +0100 +Subject: [PATCH] ping: Fix the errno handling for strtod + +The setlocale(LC_ALL, "") following the strtod() for the '-i' option +can fail if the LC_CTYPE is invalid. + +Hence the errno check following the setlocale(LC_ALL, "") thinks +wrongly that strtod() failed with the errno and prints a warning: + +$ LC_ALL=XXX ping -i 1.9 -c1 8.8.8.8 +ping: option argument contains garbage: +ping: this will become fatal error in the future +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=58 time=1.34 ms + +The errno got from the execution of strtod() is saved and restored +after setlocale() to be checked for any errors. + +The problem is only on Fedora/CentOS/RHEL with applied patch [1] +from 2012 for glibc bug #14247. + +[1] https://src.fedoraproject.org/rpms/glibc/blob/rawhide/f/glibc-rh827510.patch + +Link: https://sourceware.org/bugzilla/show_bug.cgi?id=14247 +Closes: https://github.com/iputils/iputils/pull/450 +Fixes: 918e824 ("ping: add support for sub-second timeouts") +Co-Developed-by: Sriram Rajagopalan +Reviewed-by: Petr Vorel +[ pvorel: mention glibc bug and Fedora/CentOS/RHEL ] +Signed-off-by: Sriram Rajagopalan +Signed-off-by: Jacek Tomasiak +--- + ping/ping.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/ping/ping.c b/ping/ping.c +index 89b0fa1..8f44203 100644 +--- a/ping/ping.c ++++ b/ping/ping.c +@@ -214,6 +214,7 @@ static double ping_strtod(const char *str, const char *err_msg) + { + double num; + char *end = NULL; ++ int strtod_errno = 0; + + if (str == NULL || *str == '\0') + goto err; +@@ -225,7 +226,10 @@ static double ping_strtod(const char *str, const char *err_msg) + */ + setlocale(LC_ALL, "C"); + num = strtod(str, &end); ++ strtod_errno = errno; + setlocale(LC_ALL, ""); ++ /* Ignore setlocale() errno (e.g. invalid locale in env). */ ++ errno = strtod_errno; + + if (errno || str == end || (end && *end)) { + error(0, 0, _("option argument contains garbage: %s"), end); +-- +2.33.0 + diff --git a/backport-ping-Handle-interval-correctly-in-the-second-after-b.patch b/backport-ping-Handle-interval-correctly-in-the-second-after-b.patch new file mode 100644 index 0000000..9c56a56 --- /dev/null +++ b/backport-ping-Handle-interval-correctly-in-the-second-after-b.patch @@ -0,0 +1,36 @@ +From 7448c33af407636e66ac90deb828764df51835d4 Mon Sep 17 00:00:00 2001 +From: Josh Triplett +Date: Mon, 20 Nov 2023 19:09:06 -0800 +Subject: [PATCH] ping: Handle interval correctly in the second after booting + +ping assumes that if a timespec has tv_sec == 0, it hasn't been +initialized yet. However, in the second after booting up, tv_sec will +legitimately be 0. This causes ping to send pings one after another +without waiting. + +Check that tv_nsec is 0 as well. + +Link: https://github.com/iputils/iputils/pull/499 +Reviewed-by: Petr Vorel +Tested-by: Petr Vorel +Signed-off-by: Josh Triplett +--- + ping/ping_common.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ping/ping_common.c b/ping/ping_common.c +index c8b868b..5a6c35a 100644 +--- a/ping/ping_common.c ++++ b/ping/ping_common.c +@@ -321,7 +321,7 @@ int pinger(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock) + return 1000; + + /* Check that packets < rate*time + preload */ +- if (rts->cur_time.tv_sec == 0) { ++ if (rts->cur_time.tv_sec == 0 && rts->cur_time.tv_nsec == 0) { + clock_gettime(CLOCK_MONOTONIC_RAW, &rts->cur_time); + tokens = rts->interval * (rts->preload - 1); + } else { +-- +2.33.0 + diff --git a/backport-ping.h-Remove-duplicate-include.patch b/backport-ping.h-Remove-duplicate-include.patch new file mode 100644 index 0000000..e24630e --- /dev/null +++ b/backport-ping.h-Remove-duplicate-include.patch @@ -0,0 +1,26 @@ +From bacb69e166106f0125b7288f377299894c8c7e78 Mon Sep 17 00:00:00 2001 +From: Petr Vorel +Date: Mon, 6 Mar 2023 21:17:09 +0100 +Subject: [PATCH] ping.h: Remove duplicate include + +Fixes: ba7e8a7 ("ping: merge all ping header files into a single one") +Signed-off-by: Petr Vorel +--- + ping/ping.h | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/ping/ping.h b/ping/ping.h +index caf79cd..ef358ad 100644 +--- a/ping/ping.h ++++ b/ping/ping.h +@@ -23,7 +23,6 @@ + #include + #include + #include +-#include + #include + #include + #include +-- +2.33.0 + diff --git a/backport-ping6-Fix-support-for-DSCP-Traffic-Class-option-Q.patch b/backport-ping6-Fix-support-for-DSCP-Traffic-Class-option-Q.patch new file mode 100644 index 0000000..e17207d --- /dev/null +++ b/backport-ping6-Fix-support-for-DSCP-Traffic-Class-option-Q.patch @@ -0,0 +1,54 @@ +From 425f711a62f7d7523badd6b917f15ad58ecdb0ae Mon Sep 17 00:00:00 2001 +From: Guillaume Nault +Date: Thu, 18 May 2023 18:12:54 +0200 +Subject: [PATCH] ping6: Fix support for DSCP (Traffic Class, option -Q) + +Set the IPV6_TCLASS option on probe_fd. Otherwise ip-rule is unaware +of the DSCP value at connect() time and can lookup the remote address +in the wrong routing table. + +For example: + + ip route add table main unreachable 2001:db8::10/124 + + ip route add table 100 2001:db8::10/124 dev eth0 + ip -6 rule add dsfield 0x04 table 100 + + ping -Q 0x04 2001:db8::11 + +Without this patch, probe_fd fails to connect to 2001:db8::11 (No route +to host) since the route lookup is done in the main table instead of +table 100. + +Note that, to work correctly, this patch also depends on a Linux kernel +bug fix (see +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e010ae08c71fda8be3d6bda256837795a0b3ea41). +That kernel patch has been backported to Linux stable trees and should +have already reached most distributions. + +Fixes: 33370345c7d8 ("Initial import of iputils") +Link: https://github.com/iputils/iputils/pull/468 +Reviewed-by: Petr Vorel +Signed-off-by: Guillaume Nault +--- + ping/ping6_common.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/ping/ping6_common.c b/ping/ping6_common.c +index 21333aa..e980a15 100644 +--- a/ping/ping6_common.c ++++ b/ping/ping6_common.c +@@ -182,6 +182,10 @@ int ping6_run(struct ping_rts *rts, int argc, char **argv, struct addrinfo *ai, + disable_capability_raw(); + } + ++ if (rts->tclass && ++ setsockopt(probe_fd, IPPROTO_IPV6, IPV6_TCLASS, &rts->tclass, sizeof (rts->tclass)) <0) ++ error(2, errno, "setsockopt(IPV6_TCLASS)"); ++ + if (!IN6_IS_ADDR_LINKLOCAL(&rts->firsthop.sin6_addr) && + !IN6_IS_ADDR_MC_LINKLOCAL(&rts->firsthop.sin6_addr)) + rts->firsthop.sin6_family = AF_INET6; +-- +2.33.0 + diff --git a/backport-tracepath-Don-t-assume-tv_sec-0-means-unset.patch b/backport-tracepath-Don-t-assume-tv_sec-0-means-unset.patch new file mode 100644 index 0000000..5cdffcd --- /dev/null +++ b/backport-tracepath-Don-t-assume-tv_sec-0-means-unset.patch @@ -0,0 +1,32 @@ +From c64bcd8d8eca5c7f66e75e0bc9d42828bc09ba1b Mon Sep 17 00:00:00 2001 +From: Josh Triplett +Date: Mon, 20 Nov 2023 19:15:40 -0800 +Subject: [PATCH] tracepath: Don't assume tv_sec == 0 means unset + +A CLOCK_MONOTONIC timespec's tv_sec value can legitimately be 0 during +the second after booting. Check tv_nsec as well before assuming an unset +timestamp. + +Closes: https://github.com/iputils/iputils/pull/499 +Reviewed-by: Petr Vorel +Signed-off-by: Josh Triplett +--- + tracepath.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tracepath.c b/tracepath.c +index 04d77b8..046dc33 100644 +--- a/tracepath.c ++++ b/tracepath.c +@@ -192,7 +192,7 @@ static int recverr(struct run_state *const ctl) + ctl->his[slot].hops = 0; + } + if (recv_size == sizeof(rcvbuf)) { +- if (rcvbuf.ttl == 0 || rcvbuf.ts.tv_sec == 0) ++ if (rcvbuf.ttl == 0 || (rcvbuf.ts.tv_sec == 0 && rcvbuf.ts.tv_nsec == 0)) + broken_router = 1; + else { + sndhops = rcvbuf.ttl; +-- +2.33.0 + diff --git a/iputils.spec b/iputils.spec index d5b8299..aaf560c 100644 --- a/iputils.spec +++ b/iputils.spec @@ -1,7 +1,7 @@ #needsrootforbuild Name: iputils Version: 20210722 -Release: 7 +Release: 8 Summary: Network monitoring tools including ping License: BSD and GPLv2+ URL: https://github.com/iputils/iputils @@ -26,6 +26,15 @@ Patch0009: backport-ping6-Avoid-binding-to-non-VRF.patch Patch0010: arping-Fix-exit-code-on-w-option.patch +Patch0011: backport-ping-Fix-the-errno-handling-for-strtod.patch +Patch0012: backport-ping.h-Remove-duplicate-include.patch +Patch0013: backport-clockdiff-Set-ppoll-timeout-minimum-to-1ms.patch +Patch0014: backport-ping-Fix-overflow-on-negative-i.patch +Patch0015: backport-ping6-Fix-support-for-DSCP-Traffic-Class-option-Q.patch +Patch0016: backport-ping-Handle-interval-correctly-in-the-second-after-b.patch +Patch0017: backport-tracepath-Don-t-assume-tv_sec-0-means-unset.patch +Patch0018: backport-Revert-ping-use-random-value-for-the-identifier-fiel.patch + BuildRequires: gcc meson libidn2-devel openssl-devel libcap-devel libxslt BuildRequires: docbook5-style-xsl systemd iproute glibc-kernheaders gettext %{?systemd_ordering} @@ -126,6 +135,19 @@ install -cp ifenslave.8 ${RPM_BUILD_ROOT}%{_mandir}/man8/ %{_unitdir}/ninfod.service %changelog +* Fri Jul 5 2024 cuiyudong - 20210722-8 +- Type:bugfix +- ID:NA +- SUG:NA +- DESC: ping: Fix the errno handling for strtod + ping.h: Remove duplicate include + clockdiff: Set ppoll timeout minimum to 1ms + ping: Fix overflow on negative -i + ping6: Fix support for DSCP (Traffic Class, option -Q) + ping: Handle interval correctly in the second after booting + tracepath: Don't assume tv_sec == 0 means unset + Revert "ping: use random value for the identifier field" + * Mon Mar 13 2023 eaglegai - 20210722-7 - Type:bugfix - ID:NA -- Gitee