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?

--
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;

Reply via email to