Re: git: 20b41303140e - main - ping: Print the IP options of the original packet

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