Re: [bug-inetutils] bug in bitfield macros

[Sergey Poznyakoff]

Christopher Zimmermann <address@hidden> ha escrit:

> I just discovered a small bug in ping. The macros for accessing the 
> bitfields in ping.h and the way they are called results in only one bit 
> stored per byte. I assume the desired behavior was to store 8 bits per 
> byte. So I hope this patch fixes this small issue.

Yes, indeed.  But I think it is better to be fixed in the
PING_CLR/_TST/_SET macros themselves. I have installed the attached
patch.

> It also removes a small piece of orphan code.

Thanks.

> Currently I'm working on timestamps generated by hardware or software 
> using the SO_TIMESTAMPING socket option introduced with linux 2.6.30.

How do you plan to use it? What advantages will it provide over
`ping --timestamp'?

Regards,
Sergey

Fix possible array overflow in ping.

* ping/ping.h (_C_BIT,_C_MASK)
(_PING_SET,_PING_CLR,_PING_TST): Remove in favor of
ping_common.h
* ping/ping6.h: Likewise.
* ping/ping_common.h (_C_BIT,_C_MASK): Moved from ping.h
(_C_IND): New macro.
(_PING_SET,_PING_CLR,_PING_TST): Modified versions of
macros from ping.h.
* ping/libping.c, ping/ping6.c: Update calls to _PING macros.

Index: ping/libping.c
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ping/libping.c,v
retrieving revision 1.5
diff -p -u -r1.5 libping.c
--- ping/libping.c      27 Dec 2008 20:35:57 -0000      1.5
+++ ping/libping.c      18 Aug 2009 19:11:18 -0000
@@ -117,7 +117,7 @@ ping_xmit (PING * p)
   buflen = _ping_packetsize (p);
 
   /* Mark sequence number as sent */
-  _PING_CLR (p, p->ping_num_xmit % p->ping_cktab_size);
+  _PING_CLR (p, p->ping_num_xmit);
 
   /* Encode ICMP header */
   switch (p->ping_type)
@@ -208,7 +208,7 @@ ping_recv (PING * p)
                 inet_ntoa (p->ping_from.ping_sockaddr.sin_addr));
 
       p->ping_num_recv++;
-      if (_PING_TST (p, icmp->icmp_seq % p->ping_cktab_size))
+      if (_PING_TST (p, icmp->icmp_seq))
        {
          p->ping_num_rept++;
          p->ping_num_recv--;
@@ -216,7 +216,7 @@ ping_recv (PING * p)
        }
       else
        {
-         _PING_SET (p, icmp->icmp_seq % p->ping_cktab_size);
+         _PING_SET (p, icmp->icmp_seq);
          dupflag = 0;
        }
 
Index: ping/ping.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ping/ping.h,v
retrieving revision 1.4
diff -p -u -r1.4 ping.h
--- ping/ping.h 14 Oct 2008 19:25:31 -0000      1.4
+++ ping/ping.h 18 Aug 2009 19:11:18 -0000
@@ -21,13 +21,6 @@
 
 #define USE_IPV6 0
 
-#define _C_BIT(p,bit)    (p)->ping_cktab[(bit)>>3]     /* byte in ck array */
-#define _C_MASK(bit)     (1 << ((bit) & 0x07))
-
-#define _PING_SET(p,bit) (_C_BIT (p,bit) |= _C_MASK (bit))
-#define _PING_CLR(p,bit) (_C_BIT (p,bit) &= (~_C_MASK (bit)))
-#define _PING_TST(p,bit) (_C_BIT (p,bit) & _C_MASK (bit))
-
 PING *ping_init (int type, int ident);
 void ping_reset (PING * p);
 void ping_set_type (PING * p, int type);
Index: ping/ping6.c
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ping/ping6.c,v
retrieving revision 1.18
diff -p -u -r1.18 ping6.c
--- ping/ping6.c        26 Jun 2009 14:21:20 -0000      1.18
+++ ping/ping6.c        18 Aug 2009 19:11:19 -0000
@@ -756,7 +756,7 @@ ping_xmit (PING * p)
   buflen = p->ping_datalen + sizeof (struct icmp6_hdr);
 
   /* Mark sequence number as sent */
-  _PING_CLR (p, p->ping_num_xmit % p->ping_cktab_size);
+  _PING_CLR (p, p->ping_num_xmit);
 
   icmp6 = (struct icmp6_hdr *) p->ping_buffer;
   icmp6->icmp6_type = ICMP6_ECHO_REQUEST;
@@ -835,7 +835,7 @@ ping_recv (PING * p)
       if (ntohs (icmp6->icmp6_id) != p->ping_ident)
        return -1;              /* It's not a response to us.  */
 
-      if (_PING_TST (p, ntohs (icmp6->icmp6_seq) % p->ping_cktab_size))
+      if (_PING_TST (p, ntohs (icmp6->icmp6_seq)))
        {
          /* We already got the reply for this echo request.  */
          p->ping_num_rept++;
@@ -843,7 +843,7 @@ ping_recv (PING * p)
        }
       else
        {
-         _PING_SET (p, ntohs (icmp6->icmp6_seq) % p->ping_cktab_size);
+         _PING_SET (p, ntohs (icmp6->icmp6_seq));
          p->ping_num_recv++;
          dupflag = 0;
        }
Index: ping/ping6.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ping/ping6.h,v
retrieving revision 1.6
diff -p -u -r1.6 ping6.h
--- ping/ping6.h        14 Oct 2008 19:25:31 -0000      1.6
+++ ping/ping6.h        18 Aug 2009 19:11:19 -0000
@@ -22,12 +22,6 @@
 #define PING_MAX_DATALEN (65535 - sizeof (struct icmp6_hdr))
 
 #define USE_IPV6 1
-#define _C_BIT(p,bit)    (p)->ping_cktab[(bit)>>3]     /* byte in ck array */
-#define _C_MASK(bit)     (1 << ((bit) & 0x07))
-
-#define _PING_SET(p,bit) (_C_BIT (p,bit) |= _C_MASK (bit))
-#define _PING_CLR(p,bit) (_C_BIT (p,bit) &= (~_C_MASK (bit)))
-#define _PING_TST(p,bit) (_C_BIT (p,bit) & _C_MASK (bit))
 
 static PING *ping_init (int type, int ident);
 static int ping_set_dest (PING * ping, char *host);
Index: ping/ping_common.h
===================================================================
RCS file: /cvsroot/inetutils/inetutils/ping/ping_common.h,v
retrieving revision 1.14
diff -p -u -r1.14 ping_common.h
--- ping/ping_common.h  30 Apr 2009 10:33:46 -0000      1.14
+++ ping/ping_common.h  18 Aug 2009 19:11:19 -0000
@@ -97,8 +97,8 @@ struct ping_data
   int ping_fd;                 /* Raw socket descriptor */
   int ping_type;               /* Type of packets to send */
   size_t ping_count;           /* Number of packets to send */
-  size_t ping_interval;                /* Number of seconds to wait between 
sending pkts */
-  union ping_address ping_dest;        /* whom to ping */
+  size_t ping_interval;        /* Number of seconds to wait between sending 
pkts */
+  union ping_address ping_dest;/* whom to ping */
   char *ping_hostname;         /* Printable hostname */
   size_t ping_datalen;         /* Length of data */
   int ping_ident;              /* Our identifier */
@@ -116,6 +116,28 @@ struct ping_data
   long ping_num_rept;          /* Number of duplicates received */
 };
 
+#define _C_BIT(p,bit)   (p)->ping_cktab[(bit)>>3]      /* byte in ck array */
+#define _C_MASK(bit)    (1 << ((bit) & 0x07))
+#define _C_IND(p,bit)   ((bit) % (p)->ping_cktab_size)
+
+#define _PING_SET(p,bit)                                               \
+  do                                                                   \
+    { int n = _C_IND(p,bit);                                           \
+      _C_BIT (p,n) |= _C_MASK (n);                                     \
+    }                                                                  \
+  while (0)
+
+#define _PING_CLR(p,bit)                                               \
+  do                                                                   \
+    { int n = _C_IND(p,bit);                                           \
+      _C_BIT (p,n) &= ~_C_MASK (n);                                    \
+    }                                                                  \
+  while (0)
+
+#define _PING_TST(p,bit)                                       \
+  (_C_BIT (p, _C_IND (p,bit)) & _C_MASK  (_C_IND (p,bit)))
+
+
 void tvsub (struct timeval *out, struct timeval *in);
 double nabs (double a);
 double nsqrt (double a, double prec);