On Thu, 23 Mar 2023 18:59:45 -0300
José Luis Durán <[email protected]> wrote:

> > On Mar 23, 2023, at 1:05 PM, Cy Schubert <[email protected]> wrote:
> > 
> > On Sun, 19 Mar 2023 16:33:25 GMT
> > Mark Johnston <[email protected]> wrote:
> >   
> >> The branch main has been updated by markj:
> >> 
> >> URL: 
> >> https://cgit.FreeBSD.org/src/commit/?id=20b41303140eee4dfb896558fb83600c5f013d39
> >> 
> >> commit 20b41303140eee4dfb896558fb83600c5f013d39
> >> Author:     Jose Luis Duran <[email protected]>
> >> AuthorDate: 2023-02-09 15:47:53 +0000
> >> Commit:     Mark Johnston <[email protected]>
> >> 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 <[email protected]>
> > FreeBSD UNIX:  <[email protected]>   Web:  https://FreeBSD.org
> > NTP:           <[email protected]>    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 <[email protected]>
FreeBSD UNIX:  <[email protected]>   Web:  https://FreeBSD.org
NTP:           <[email protected]>    Web:  https://nwtime.org

                        e^(i*pi)+1=0

Reply via email to