Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 23 Mar 2023 13:48:52 -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:  <6AEF84FA-8055-4AD1-83F1-D89386C33092@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
> 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.

Right. That is strange it happened, but I=E2=80=99ll submit a fix. Thank =
you!

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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6AEF84FA-8055-4AD1-83F1-D89386C33092>