On Tue, Apr 10, 2018 at 11:11:39PM +0300, Vadim Zhukov wrote:
> Hi all!
>
> While working on home job for students, I've come across two
> questionable thingies in ping.c:
>
> 1. It sends process PID (well, last 16 bits) to the network.
> Maybe I'm a bit paranoid, but this looks like bad idea for me.
> I understand that this worked good when PIDs were 16-bit limited,
> because in that case you'll get 100% guarantee two different
> ping processes won't overlap. But nowadays we have larger PID
> space, so clashes are possible anyway. I propose to go straight
> with arc4random().
>
> 2. There is no point in calling ntohs/htons on the ident value.
> Or we should do this in IPv4 too, then: I'm not opposed, but
> at least consistency should be honored, IMHO.
>
> Okay? Comments?
OK florian@
>
> --
> WBR,
> Vadim Zhukov
>
> P.S.: I'm not fully back yet, sorry.
>
>
> Index: ping.c
> ===================================================================
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.224
> diff -u -p -r1.224 ping.c
> --- ping.c 8 Nov 2017 17:27:39 -0000 1.224
> +++ ping.c 10 Apr 2018 20:03:50 -0000
> @@ -586,7 +586,7 @@ main(int argc, char *argv[])
> for (i = ECHOTMLEN; i < datalen; ++i)
> *datap++ = i;
>
> - ident = getpid() & 0xFFFF;
> + ident = arc4random() & 0xFFFF;
>
> /*
> * When trying to send large packets, you must increase the
> @@ -1033,7 +1033,7 @@ pinger(int s)
> icp6->icmp6_cksum = 0;
> icp6->icmp6_type = ICMP6_ECHO_REQUEST;
> icp6->icmp6_code = 0;
> - icp6->icmp6_id = htons(ident);
> + icp6->icmp6_id = ident;
> icp6->icmp6_seq = seq;
> } else {
> icp = (struct icmp *)outpack;
> @@ -1151,7 +1151,7 @@ pr_pack(u_char *buf, int cc, struct msgh
> }
>
> if (icp6->icmp6_type == ICMP6_ECHO_REPLY) {
> - if (ntohs(icp6->icmp6_id) != ident)
> + if (icp6->icmp6_id != ident)
> return; /* 'Twas not our ECHO */
> seq = icp6->icmp6_seq;
> echo_reply = 1;
>
--
I'm not entirely sure you are real.
