From nobody Thu Mar 23 21:59:45 2023 X-Original-To: dev-commits-src-all@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 4PjK5D5MNmz40NwH; Thu, 23 Mar 2023 22:00:00 +0000 (UTC) (envelope-from jlduran@gmail.com) Received: from mail-oi1-x229.google.com (mail-oi1-x229.google.com [IPv6:2607:f8b0:4864:20::229]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PjK5D2XJGz4KhL; Thu, 23 Mar 2023 22:00:00 +0000 (UTC) (envelope-from jlduran@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-oi1-x229.google.com with SMTP id bj20so7850085oib.3; Thu, 23 Mar 2023 15:00:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679608799; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=g+U+F3EVQA6x9HkRjJMIVzozRcge5PDPFQXKkoARiXw=; b=TJEjy2b9qwZT9Ccn/H4Rca5meQHsUpuK72jAsMwfc4RUXu4ZklR9hph5jBY845ISpw qutgkhnsSmr0HxHhmNYkAgS4VGLKcLgOJTJZ+CgZxONozx3DX3taRiTvBYyWayZplMQZ CTjg+e88dIn05TB2zhHMJT8TLi/XcXrMCs3tZQt2B4kqD3w9B4yNQQ+864DLDMZanzBf um2lhAgrbvOLQlYg5GXPBCmSDs67CWAe/CoB2m3F3EZjZftCkHuyWwveYzy3geUptAqE K/qmTP0K+wvb77N/gZc7/FuvHRYXjeXG5grpbyKgsKCqKuTWpgEepoqYjax2fo6Hbjt+ cs7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679608799; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=g+U+F3EVQA6x9HkRjJMIVzozRcge5PDPFQXKkoARiXw=; b=E7p9rYRxZr2WUNd0J4tG3JUW6DoHV70VW0TIdtVpNENy7t1guiwmQ3jkYjCIHZ0RKl 5M5tMkYQnLuuL2EZu7t5Ii9MyWdurVz7Stn8d0vuKds/TxBVu/P8oglsnlDCnbt5howo NYBklZ3AZCXvk1Zh/NqyyP0jOUm4nGQYCO1QFUAd3HWzSQtYcy6RIfNlCe2EXEA03FOv n3+ak6uCdTyYtXlMzQChqU0qc5Hcs1i7OBI0/li1uv+MfYI/bU9kSKW0GNU6pf9aKOVW /zSTvg+1A5UubHQ1S2skEjGI4yUqsf6+jitx8f80OuJ8ZBK0M5icil8VvN3vMDKSZ5r0 YBtg== X-Gm-Message-State: AO0yUKUqBAbt5Nk6EZcjMffbQzVh7V9E4IYwThjB2yLqzLfiHeSIRwJW baWKzXFxRVJwjuL50TVBWe/X2CTs6qo= X-Google-Smtp-Source: AK7set9ntN4+0Mv4vte/EBRrYNr3Bohar3fkj4+EiQm1K7NCZIi6b09i1cWsE/gq4+TaZe8hLWzIfQ== X-Received: by 2002:a05:6808:424e:b0:384:2a24:bd7e with SMTP id dp14-20020a056808424e00b003842a24bd7emr63884oib.44.1679608799109; Thu, 23 Mar 2023 14:59:59 -0700 (PDT) Received: from smtpclient.apple (mlsrj200152099p037.mls.com.br. [200.152.99.37]) by smtp.gmail.com with ESMTPSA id q6-20020acad906000000b003876369bd0asm917390oig.19.2023.03.23.14.59.57 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Mar 2023 14:59:58 -0700 (PDT) From: =?utf-8?B?Sm9zw6kgTHVpcyBEdXLDoW4=?= Message-Id: <8FFC2AAC-A06E-4791-9C33-AF0D3A26529F@gmail.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_93831C93-95ED-479E-BA26-94F08F218FFC" List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\)) Subject: Re: git: 20b41303140e - main - ping: Print the IP options of the original packet Date: Thu, 23 Mar 2023 18:59:45 -0300 In-Reply-To: <20230323090520.30554030@slippy> Cc: Mark Johnston , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" To: Cy Schubert References: <202303191633.32JGXPov003163@gitrepo.freebsd.org> <20230323090520.30554030@slippy> X-Mailer: Apple Mail (2.3731.400.51.1.1) X-Rspamd-Queue-Id: 4PjK5D2XJGz4KhL X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --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 = wrote: >=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=3D20b41303140eee4dfb896558fb83600c= 5f013d39 >>=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 = 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 > FreeBSD UNIX: Web: https://FreeBSD.org > NTP: 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
On Mar 23, 2023, = at 1:05 PM, Cy Schubert <Cy.Schubert@cschubert.com> = wrote:

On Sun, 19 = Mar 2023 16:33:25 GMT
Mark Johnston <markj@FreeBSD.org> = wrote:

The branch main has been updated = by markj:

URL: = https://cgit.FreeBSD.org/src/commit/?id=3D20b41303140eee4dfb896558fb83600c= 5f013d39

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

   ping: Print the IP options of the = original packet

   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().

=    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.

=    Pass the raw ICMP data along with the IP struct, in = order to print the
   options, if any.

=    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(-)

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;

/*
* 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)
= = }

= = memcpy(&oip, icmp_data_raw, sizeof(struct ip));
- oicmp_raw = =3D icmp_data_raw + oip_header_len;
- memcpy(&oicmp, oicmp_raw, = sizeof(struct icmp));

Coverity is complaining about = uninitialized oicmp due to the deletion
of the above two = lines.


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;

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 */

= = (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;

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
=             &n= bsp;   "stderr": "",
=             &n= bsp;   "redacted": False,
=             },=
- =            marks=3D= pytest.mark.skip("XXX currently failing"),
=             id= =3D"_3_1_opts_NOP_40",
=         ),
=         pytest.param(




--
Cheers,
Cy Schubert = <Cy.Schubert@cschubert.com>
FreeBSD UNIX: =  <cy@FreeBSD.org>   Web: =  https://FreeBSD.org
NTP: =           <cy@nwtime.= org>    Web:  https://nwtime.org

= e^(i*pi)+1=3D0

It has = been submitted under https://reviews.freebsd.org/D3= 9233
= --Apple-Mail=_93831C93-95ED-479E-BA26-94F08F218FFC--