From nobody Thu Mar 23 22:12:03 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4PjKMD1cXqz40Pv7; Thu, 23 Mar 2023 22:12:08 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from omta001.cacentral1.a.cloudfilter.net (omta001.cacentral1.a.cloudfilter.net [3.97.99.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PjKMC6bdvz4NpQ; Thu, 23 Mar 2023 22:12:07 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Authentication-Results: mx1.freebsd.org; none Received: from shw-obgw-4003a.ext.cloudfilter.net ([10.228.9.183]) by cmsmtp with ESMTP id fPnIpYmikuZMSfTAEpNl0g; Thu, 23 Mar 2023 22:12:06 +0000 Received: from spqr.komquats.com ([70.66.148.124]) by cmsmtp with ESMTPA id fTACpvzk7cyvufTADpMqDC; Thu, 23 Mar 2023 22:12:06 +0000 X-Authority-Analysis: v=2.4 cv=VbHkgXl9 c=1 sm=1 tr=0 ts=641cceb6 a=Cwc3rblV8FOMdVN/wOAqyQ==:117 a=Cwc3rblV8FOMdVN/wOAqyQ==:17 a=xqWC_Br6kY4A:10 a=IkcTkHD0fZMA:10 a=k__wU0fu6RkA:10 a=pGLkceISAAAA:8 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=EkcXrb_YAAAA:8 a=BgOcHarDN3n17w1KK4kA:9 a=QEXdDO2ut3YA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 a=LK5xJRSDVpKd5WXXoEvA:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTP id 6BE852549; Thu, 23 Mar 2023 15:12:04 -0700 (PDT) Received: from slippy (localhost [IPv6:::1]) by slippy.cwsent.com (Postfix) with ESMTP id 36759842; Thu, 23 Mar 2023 15:12:04 -0700 (PDT) Date: Thu, 23 Mar 2023 15:12:03 -0700 From: Cy Schubert To: =?ISO-8859-1?Q?Jos=E9?= Luis =?ISO-8859-1?Q?Dur=E1n?= Cc: Mark Johnston , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 20b41303140e - main - ping: Print the IP options of the original packet Message-ID: <20230323151203.20f31a9f@slippy> In-Reply-To: <8FFC2AAC-A06E-4791-9C33-AF0D3A26529F@gmail.com> References: <202303191633.32JGXPov003163@gitrepo.freebsd.org> <20230323090520.30554030@slippy> <8FFC2AAC-A06E-4791-9C33-AF0D3A26529F@gmail.com> Organization: KOMQUATS X-Mailer: Claws Mail 3.19.0 (GTK+ 2.24.33; amd64-portbld-freebsd14.0) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-CMAE-Envelope: MS4xfA84GHqgtH9mgAJYWm61p+q+eSqT2laGQio4ZFnRflCjKzzPWrU3haQHADt1/jcLbVtfoR5zIHwdfegcCqZZHlJunMpZkHWRQMy4NGXfZRtYQ+AFwxJG t1abcX+KUxV6iLUX9b5lVltekDHQf+s1mfWvAA6EbWEK7VC3vCZi2pXLNcyKPuGsg2/dh8UbE8hyRoEbhAq0+KhE7yrtivLWIXYxSWFOVQxq4sw9w9xjxz2g uViuxUAH3OhbYiFMZhgENCgYVf0bMBZYKi3M68IyhAUQmxLt74/VJqh112+21z5uelYxdiBofnn69KhW6M7Wnwl8Cp4sAM430GBJc01CYDBpF0m7B1WvdgGO bN4TLafg X-Rspamd-Queue-Id: 4PjKMC6bdvz4NpQ X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:16509, ipnet:3.96.0.0/15, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On Thu, 23 Mar 2023 18:59:45 -0300 Jos=C3=A9 Luis Dur=C3=A1n wrote: > > On Mar 23, 2023, at 1:05 PM, Cy Schubert wr= ote: > >=20 > > On Sun, 19 Mar 2023 16:33:25 GMT > > Mark Johnston wrote: > > =20 > >> The branch main has been updated by markj: > >>=20 > >> URL: https://cgit.FreeBSD.org/src/commit/?id=3D20b41303140eee4dfb89655= 8fb83600c5f013d39 > >>=20 > >> commit 20b41303140eee4dfb896558fb83600c5f013d39 > >> Author: Jose Luis Duran > >> AuthorDate: 2023-02-09 15:47:53 +0000 > >> Commit: Mark Johnston > >> CommitDate: 2023-03-19 16:23:06 +0000 > >>=20 > >> ping: Print the IP options of the original packet > >>=20 > >> When an ICMP packet contains an IP packet in its payload, and that > >> original IP packet contains options, these options were not display= ed > >> accordingly in pr_iph(). > >>=20 > >> pr_iph() is a function that prints the original "quoted packet" IP > >> header, with only an IP struct as an argument. The IP struct does = not > >> contain IP options, and it is not guaranteed that the options will = be > >> contiguous in memory to the IP struct after > >> d9cacf605e2ac0f704e1ce76357cbfbe6cb63d52. > >>=20 > >> Pass the raw ICMP data along with the IP struct, in order to print = the > >> options, if any. > >>=20 > >> Reviewed by: markj > >> MFC after: 1 week > >> Differential Revision: https://reviews.freebsd.org/D38469 > >> --- > >> sbin/ping/ping.c | 19 ++++++++++--------- > >> sbin/ping/tests/ping_test.sh | 2 +- > >> sbin/ping/tests/test_ping.py | 1 - > >> 3 files changed, 11 insertions(+), 11 deletions(-) > >>=20 > >> diff --git a/sbin/ping/ping.c b/sbin/ping/ping.c > >> index fe319bfb70a5..b1721ad72a5c 100644 > >> --- a/sbin/ping/ping.c > >> +++ b/sbin/ping/ping.c > >> @@ -225,7 +225,7 @@ static void pinger(void); > >> static char *pr_addr(struct in_addr); > >> static char *pr_ntime(n_time); > >> static void pr_icmph(struct icmp *, struct ip *, const u_char *const); > >> -static void pr_iph(struct ip *); > >> +static void pr_iph(struct ip *, const u_char *); > >> static void pr_pack(char *, ssize_t, struct sockaddr_in *, struct time= spec *); > >> static void pr_retip(struct ip *, const u_char *); > >> static void status(int); > >> @@ -1157,7 +1157,6 @@ pr_pack(char *buf, ssize_t cc, struct sockaddr_i= n *from, struct timespec *tv) > >> struct ip oip; > >> u_char oip_header_len; > >> struct icmp oicmp; > >> - const u_char *oicmp_raw; > >>=20 > >> /* > >> * Get size of IP header of the received packet. > >> @@ -1355,8 +1354,6 @@ pr_pack(char *buf, ssize_t cc, struct sockaddr_i= n *from, struct timespec *tv) > >> } > >>=20 > >> memcpy(&oip, icmp_data_raw, sizeof(struct ip)); > >> - oicmp_raw =3D icmp_data_raw + oip_header_len; > >> - memcpy(&oicmp, oicmp_raw, sizeof(struct icmp)); =20 > >=20 > > Coverity is complaining about uninitialized oicmp due to the deletion > > of the above two lines. > > =20 > >>=20 > >> if (((options & F_VERBOSE) && uid =3D=3D 0) || > >> (!(options & F_QUIET2) && > >> @@ -1366,7 +1363,7 @@ pr_pack(char *buf, ssize_t cc, struct sockaddr_i= n *from, struct timespec *tv) > >> (oicmp.icmp_id =3D=3D ident))) { > >> (void)printf("%zd bytes from %s: ", cc, > >> pr_addr(from->sin_addr)); > >> - pr_icmph(&icp, &oip, oicmp_raw); > >> + pr_icmph(&icp, &oip, icmp_data_raw); > >> } else > >> return; > >> } > >> @@ -1663,14 +1660,13 @@ pr_icmph(struct icmp *icp, struct ip *oip, con= st u_char *const oicmp_raw) > >> * Print an IP header with options. > >> */ > >> static void > >> -pr_iph(struct ip *ip) > >> +pr_iph(struct ip *ip, const u_char *cp) > >> { > >> struct in_addr ina; > >> - u_char *cp; > >> int hlen; > >>=20 > >> hlen =3D ip->ip_hl << 2; > >> - cp =3D (u_char *)ip + sizeof(struct ip); /* point to options */ > >> + cp =3D cp + sizeof(struct ip); /* point to options */ > >>=20 > >> (void)printf("Vr HL TOS Len ID Flg off TTL Pro cks Src = Dst\n"); > >> (void)printf(" %1x %1x %02x %04x %04x", > >> @@ -1723,7 +1719,12 @@ pr_addr(struct in_addr ina) > >> static void > >> pr_retip(struct ip *ip, const u_char *cp) > >> { > >> - pr_iph(ip); > >> + int8_t hlen; > >> + > >> + pr_iph(ip, cp); > >> + > >> + hlen =3D ip->ip_hl << 2; > >> + cp =3D cp + hlen; > >>=20 > >> if (ip->ip_p =3D=3D 6) > >> (void)printf("TCP: from port %u, to port %u (decimal)\n", > >> diff --git a/sbin/ping/tests/ping_test.sh b/sbin/ping/tests/ping_test.= sh > >> index 9f821ed96360..53383f2fb7bc 100644 > >> --- a/sbin/ping/tests/ping_test.sh > >> +++ b/sbin/ping/tests/ping_test.sh > >> @@ -178,7 +178,7 @@ inject_pip_head() > >> } > >> inject_pip_body() > >> { > >> - atf_check -s exit:2 -o match:"Destination Host Unreachable" -o not-m= atch:"01010101" python3 $(atf_get_srcdir)/injection.py pip > >> + atf_check -s exit:2 -o match:"Destination Host Unreachable" -o match= :"(01){40}" python3 $(atf_get_srcdir)/injection.py pip > >> } > >> inject_pip_cleanup() > >> { > >> diff --git a/sbin/ping/tests/test_ping.py b/sbin/ping/tests/test_ping.= py > >> index a9b760ff7aca..79fd332f1c18 100644 > >> --- a/sbin/ping/tests/test_ping.py > >> +++ b/sbin/ping/tests/test_ping.py > >> @@ -893,7 +893,6 @@ Vr HL TOS Len ID Flg off TTL Pro cks Src= Dst > >> "stderr": "", > >> "redacted": False, > >> }, > >> - marks=3Dpytest.mark.skip("XXX currently failing"), > >> id=3D"_3_1_opts_NOP_40", > >> ), > >> pytest.param( > >> =20 > >=20 > >=20 > >=20 > > --=20 > > Cheers, > > Cy Schubert > > FreeBSD UNIX: Web: https://FreeBSD.org > > NTP: Web: https://nwtime.org > >=20 > > e^(i*pi)+1=3D0 =20 >=20 > It has been submitted under https://reviews.freebsd.org/D39233 Thanks. I got the email from phabricator and left a commment. --=20 Cheers, Cy Schubert FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org e^(i*pi)+1=3D0