Date: Thu, 23 Mar 2023 18:59:45 -0300 From: =?utf-8?B?Sm9zw6kgTHVpcyBEdXLDoW4=?= <jlduran@gmail.com> To: Cy Schubert <Cy.Schubert@cschubert.com> Cc: Mark Johnston <markj@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 20b41303140e - main - ping: Print the IP options of the original packet Message-ID: <8FFC2AAC-A06E-4791-9C33-AF0D3A26529F@gmail.com> In-Reply-To: <20230323090520.30554030@slippy> References: <202303191633.32JGXPov003163@gitrepo.freebsd.org> <20230323090520.30554030@slippy>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_93831C93-95ED-479E-BA26-94F08F218FFC Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On Mar 23, 2023, at 1:05 PM, Cy Schubert <Cy.Schubert@cschubert.com> = wrote: >=20 > On Sun, 19 Mar 2023 16:33:25 GMT > Mark Johnston <markj@FreeBSD.org> wrote: >=20 >> The branch main has been updated by markj: >>=20 >> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D20b41303140eee4dfb896558fb83600c= 5f013d39 >>=20 >> commit 20b41303140eee4dfb896558fb83600c5f013d39 >> Author: Jose Luis Duran <jlduran@gmail.com> >> AuthorDate: 2023-02-09 15:47:53 +0000 >> Commit: Mark Johnston <markj@FreeBSD.org> >> 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 = displayed >> 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 = timespec *); >> 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_in *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_in *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 > 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_in *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, = const 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-match:"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 <Cy.Schubert@cschubert.com> > FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org > NTP: <cy@nwtime.org> Web: https://nwtime.org >=20 > e^(i*pi)+1=3D0 It has been submitted under https://reviews.freebsd.org/D39233= --Apple-Mail=_93831C93-95ED-479E-BA26-94F08F218FFC Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii <html><head><meta http-equiv=3D"content-type" content=3D"text/html; = charset=3Dus-ascii"></head><body style=3D"overflow-wrap: break-word; = -webkit-nbsp-mode: space; line-break: = after-white-space;"><div><blockquote type=3D"cite"><div>On Mar 23, 2023, = at 1:05 PM, Cy Schubert <Cy.Schubert@cschubert.com> = wrote:</div><br class=3D"Apple-interchange-newline"><div><div>On Sun, 19 = Mar 2023 16:33:25 GMT<br>Mark Johnston <markj@FreeBSD.org> = wrote:<br><br><blockquote type=3D"cite">The branch main has been updated = by markj:<br><br>URL: = https://cgit.FreeBSD.org/src/commit/?id=3D20b41303140eee4dfb896558fb83600c= 5f013d39<br><br>commit = 20b41303140eee4dfb896558fb83600c5f013d39<br>Author: = Jose Luis Duran = <jlduran@gmail.com><br>AuthorDate: 2023-02-09 15:47:53 = +0000<br>Commit: Mark Johnston = <markj@FreeBSD.org><br>CommitDate: 2023-03-19 16:23:06 = +0000<br><br> ping: Print the IP options of the = original packet<br><br> When an ICMP packet contains = an IP packet in its payload, and that<br> original IP = packet contains options, these options were not displayed<br> = accordingly in pr_iph().<br><br> = pr_iph() is a function that prints the original = "quoted packet" IP<br> header, with only an IP struct = as an argument. The IP struct does not<br> = contain IP options, and it is not guaranteed that the = options will be<br> contiguous in memory to the IP = struct after<br> = d9cacf605e2ac0f704e1ce76357cbfbe6cb63d52.<br><br> = Pass the raw ICMP data along with the IP struct, in = order to print the<br> options, if any.<br><br> = Reviewed by: markj<br> = MFC after: 1 week<br> = Differential Revision: = https://reviews.freebsd.org/D38469<br>---<br>; sbin/ping/ping.c = | = 19 ++++++++++---------<br> sbin/ping/tests/ping_test.sh | 2 +-<br> = sbin/ping/tests/test_ping.py | 1 -<br> 3 files changed, 11 = insertions(+), 11 deletions(-)<br><br>diff --git a/sbin/ping/ping.c = b/sbin/ping/ping.c<br>index fe319bfb70a5..b1721ad72a5c 100644<br>--- = a/sbin/ping/ping.c<br>+++ b/sbin/ping/ping.c<br>@@ -225,7 +225,7 @@ = static void pinger(void);<br> static char *pr_addr(struct in_addr);<br> = static char *pr_ntime(n_time);<br> static void pr_icmph(struct icmp *, = struct ip *, const u_char *const);<br>-static void pr_iph(struct ip = *);<br>+static void pr_iph(struct ip *, const u_char *);<br> static void = pr_pack(char *, ssize_t, struct sockaddr_in *, struct timespec *);<br> = static void pr_retip(struct ip *, const u_char *);<br> static void = status(int);<br>@@ -1157,7 +1157,6 @@ pr_pack(char *buf, ssize_t cc, = struct sockaddr_in *from, struct timespec *tv)<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>struct ip = oip;<br> <span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>u_char oip_header_len;<br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>struct icmp oicmp;<br>-<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>const = u_char *oicmp_raw;<br><br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>/*<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span> * Get = size of IP header of the received packet.<br>@@ -1355,8 +1354,6 @@ = pr_pack(char *buf, ssize_t cc, struct sockaddr_in *from, struct timespec = *tv)<br> <span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>}<br><br> <span class=3D"Apple-tab-span" style=3D"white-space:pre">= </span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>memcpy(&oip, icmp_data_raw, sizeof(struct ip));<br>-<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>oicmp_raw = =3D icmp_data_raw + oip_header_len;<br>-<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>memcpy(&oicmp, oicmp_raw, = sizeof(struct icmp));<br></blockquote><br>Coverity is complaining about = uninitialized oicmp due to the deletion<br>of the above two = lines.<br><br><blockquote type=3D"cite"><br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>if = (((options & F_VERBOSE) && uid =3D=3D 0) ||<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span> = (!(options & F_QUIET2) &&<br>@@ -1366,7 = +1363,7 @@ pr_pack(char *buf, ssize_t cc, struct sockaddr_in *from, = struct timespec *tv)<br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span> = (oicmp.icmp_id =3D=3D ident))) {<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span> = (void)printf("%zd bytes from %s: ", cc,<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>pr_addr(from->sin_addr));<br>-<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span> = pr_icmph(&icp, &oip, oicmp_raw);<br>+<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span> = pr_icmph(&icp, &oip, icmp_data_raw);<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>} = else<br> <span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span> return;<br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>}<br>@@ -1663,14 +1660,13 @@ = pr_icmph(struct icmp *icp, struct ip *oip, const u_char *const = oicmp_raw)<br> *<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>Print an IP header with = options.<br> */<br> static void<br>-pr_iph(struct ip = *ip)<br>+pr_iph(struct ip *ip, const u_char *cp)<br> {<br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>struct = in_addr ina;<br>-<span class=3D"Apple-tab-span" style=3D"white-space:pre">= </span>u_char *cp;<br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>int hlen;<br><br> <span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>hlen =3D = ip->ip_hl << 2;<br>-<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>cp =3D (u_char *)ip + = sizeof(struct ip);<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>/* point to options */<br>+<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>cp =3D cp = + sizeof(struct ip);<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span><span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>/* point to options */<br><br> = <span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>(void)printf("Vr HL TOS Len ID Flg off = TTL Pro cks Src = Dst\n");<br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>(void)printf(" %1x %1x = %02x %04x %04x",<br>@@ -1723,7 +1719,12 @@ pr_addr(struct in_addr = ina)<br> static void<br> pr_retip(struct ip *ip, const u_char *cp)<br> = {<br>-<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>pr_iph(ip);<br>+<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>int8_t hlen;<br>+<br>+<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>pr_iph(ip, cp);<br>+<br>+<span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>hlen =3D ip->ip_hl << = 2;<br>+<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>cp =3D cp + hlen;<br><br> <span class=3D"Apple-tab-span" = style=3D"white-space:pre"> </span>if (ip->ip_p =3D=3D 6)<br> = <span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span><span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>(void)printf("TCP: from port %u, to port %u (decimal)\n",<br>diff = --git a/sbin/ping/tests/ping_test.sh = b/sbin/ping/tests/ping_test.sh<br>index 9f821ed96360..53383f2fb7bc = 100644<br>--- a/sbin/ping/tests/ping_test.sh<br>+++ = b/sbin/ping/tests/ping_test.sh<br>@@ -178,7 +178,7 @@ = inject_pip_head()<br> }<br> inject_pip_body()<br> {<br>-<span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span>atf_check = -s exit:2 -o match:"Destination Host Unreachable" -o = not-match:"01010101" python3 $(atf_get_srcdir)/injection.py = pip<br>+<span class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>atf_check -s exit:2 -o match:"Destination Host Unreachable" -o = match:"(01){40}" python3 $(atf_get_srcdir)/injection.py pip<br> }<br> = inject_pip_cleanup()<br> {<br>diff --git a/sbin/ping/tests/test_ping.py = b/sbin/ping/tests/test_ping.py<br>index a9b760ff7aca..79fd332f1c18 = 100644<br>--- a/sbin/ping/tests/test_ping.py<br>+++ = b/sbin/ping/tests/test_ping.py<br>@@ -893,7 +893,6 @@ Vr HL TOS = Len ID Flg off TTL Pro cks = Src Dst<br> = &n= bsp; "stderr": "",<br> = &n= bsp; "redacted": False,<br> = },= <br>- = marks=3D= pytest.mark.skip("XXX currently failing"),<br> = id= =3D"_3_1_opts_NOP_40",<br> = ),<br> = pytest.param(<br><br></blo= ckquote><br><br><br>-- <br>Cheers,<br>Cy Schubert = <Cy.Schubert@cschubert.com><br>FreeBSD UNIX: = <cy@FreeBSD.org> Web: = https://FreeBSD.org<br>NTP: = <cy@nwtime.= org> Web: https://nwtime.org<br><br><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> </span><span = class=3D"Apple-tab-span" style=3D"white-space:pre"> = </span>e^(i*pi)+1=3D0<br></div></div></blockquote></div><br><div>It has = been submitted under <a = href=3D"https://reviews.freebsd.org/D39233">https://reviews.freebsd.org/D3= 9233</a></div></body></html>= --Apple-Mail=_93831C93-95ED-479E-BA26-94F08F218FFC--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8FFC2AAC-A06E-4791-9C33-AF0D3A26529F>