Re: git: 20b41303140e - main - ping: Print the IP options of the original packet
- In reply to: José Luis Durán : "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 22:12:03 UTC
On Thu, 23 Mar 2023 18:59:45 -0300
José Luis Durán <jlduran@gmail.com> wrote:
> > 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.
> >
> >>
> >> 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
>
> It has been submitted under https://reviews.freebsd.org/D39233
Thanks. I got the email from phabricator and left a commment.
--
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