Re: git: 20b41303140e - main - ping: Print the IP options of the original packet
- In reply to: Cy Schubert : "Re: git: 20b41303140e - main - ping: Print the IP options of the original packet"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 23 Mar 2023 16:48:52 UTC
> 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=20b41303140eee4dfb896558fb83600c5f013d39
>>
>> 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 = 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.
Right. That is strange it happened, but I’ll submit a fix. Thank you!
>
>>
>> if (((options & F_VERBOSE) && uid == 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 == 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 = ip->ip_hl << 2;
>> - cp = (u_char *)ip + sizeof(struct ip); /* point to options */
>> + cp = 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 = ip->ip_hl << 2;
>> + cp = cp + hlen;
>>
>> if (ip->ip_p == 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=pytest.mark.skip("XXX currently failing"),
>> id="_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=0