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;