From: Amos Jeffries <squid3@treenet.co.nz>
Date: Tue, 22 Jan 2013 04:39:33 +0000 (-0700)
Subject: WCCP: Fix memory leak in mask assignment, improve debuggsing.
X-Git-Tag: SQUID_3_3_1~35
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=216616a6460faee02ec89020e5d49fe27533a6d4;p=thirdparty%2Fsquid.git

WCCP: Fix memory leak in mask assignment, improve debuggsing.

* Release temporary weight array allocated on each HERE_I_AM packet sent
  by Squid. For mask assignment these were not released properly and may
  build up to a significant size of memory over time.

* Add debug traces to send() events to report failures sending packets

* Also, on HERE_I_AM event send() failure, reduce the timeout to 2sec
  for the retry in a crude attempt to prevent router state flapping.

* Silence compiler warnings on use of connect() to disconnect a socket.
  Inconsistent OS behaviour makes the result useless in this case.

 Detected by Coverity Scan. Issues 740329, 740330, 740331, 740332,
    740333, 740441.
---

diff --git a/src/defines.h b/src/defines.h
index a55b8dbd18..3c42025742 100644
--- a/src/defines.h
+++ b/src/defines.h
@@ -173,6 +173,11 @@
 #define IPC_UNIX_STREAM 4
 #define IPC_UNIX_DGRAM 5
 
+/* required for AF_UNIX below to be defined [on FreeBSD] */
+#if HAVE_SYS_SOCKET_H
+#include <sys/socket.h> 
+#endif
+
 #if HAVE_SOCKETPAIR && defined (AF_UNIX)
 #define IPC_STREAM IPC_UNIX_STREAM
 #define IPC_DGRAM IPC_UNIX_DGRAM
diff --git a/src/wccp.cc b/src/wccp.cc
index b43cdb4e57..941ad126f7 100644
--- a/src/wccp.cc
+++ b/src/wccp.cc
@@ -297,13 +297,18 @@ wccpHereIam(void *voidnotused)
     debugs(80, 6, "wccpHereIam: Called");
 
     wccp_here_i_am.id = last_id;
-    comm_udp_send(theWccpConnection,
-                  &wccp_here_i_am,
-                  sizeof(wccp_here_i_am),
-                  0);
+    double interval = 10.0; // TODO: make this configurable, possibly negotiate with the router.
+    errno = 0;
+    ssize_t sent = comm_udp_send(theWccpConnection, &wccp_here_i_am, sizeof(wccp_here_i_am), 0);
+
+    // if we failed to send the whole lot, try again at a shorter interval (20%)
+    if (sent != sizeof(wccp_here_i_am)) {
+        debugs(80, 2, "ERROR: failed to send WCCP HERE_I_AM packet: " << xstrerror());
+        interval = 2.0;
+    }
 
     if (!eventFind(wccpHereIam, NULL))
-        eventAdd("wccpHereIam", wccpHereIam, NULL, 10.0, 1);
+        eventAdd("wccpHereIam", wccpHereIam, NULL, interval, 1);
 }
 
 static void
diff --git a/src/wccp2.cc b/src/wccp2.cc
index 6cb126d8fd..b7da8b7fa0 100644
--- a/src/wccp2.cc
+++ b/src/wccp2.cc
@@ -1000,7 +1000,8 @@ wccp2ConnectionOpen(void)
 #if defined(IP_MTU_DISCOVER) && defined(IP_PMTUDISC_DONT)
     {
         int i = IP_PMTUDISC_DONT;
-        setsockopt(theWccp2Connection, SOL_IP, IP_MTU_DISCOVER, &i, sizeof i);
+        if (setsockopt(theWccp2Connection, SOL_IP, IP_MTU_DISCOVER, &i, sizeof i) < 0)
+            debugs(80, 2, "WARNING: Path MTU discovery could not be disabled on FD " << theWccp2Connection << ": " << xstrerror());
     }
 
 #endif
@@ -1039,8 +1040,9 @@ wccp2ConnectionOpen(void)
             /* Disconnect the sending socket. Note: FreeBSD returns error
              * but disconnects anyway so we have to just assume it worked
              */
-            if (wccp2_numrouters > 1)
-                connect(theWccp2Connection, (struct sockaddr *) &null, router_len);
+            if (wccp2_numrouters > 1) {
+                (void)connect(theWccp2Connection, (struct sockaddr *) &null, router_len);
+            }
         }
 
         service_list_ptr = service_list_ptr->next;
@@ -1598,10 +1600,9 @@ wccp2HereIam(void *voidnotused)
                                 &service_list_ptr->wccp_packet,
                                 service_list_ptr->wccp_packet_size);
             } else {
-                send(theWccp2Connection,
-                     &service_list_ptr->wccp_packet,
-                     service_list_ptr->wccp_packet_size,
-                     0);
+                errno = 0;
+                if (send(theWccp2Connection, &service_list_ptr->wccp_packet, service_list_ptr->wccp_packet_size, 0) < service_list_ptr->wccp_packet_size)
+                    debugs(80, 2, "ERROR: failed to send WCCPv2 HERE_I_AM packet to " << router << " : " << xstrerror());
             }
         }
 
@@ -1975,20 +1976,21 @@ wccp2AssignBuckets(void *voidnotused)
             if (ntohl(router_list_ptr->num_caches)) {
                 /* send packet */
 
+                /* FIXME INET6 : drop temp conversion */
+                Ip::Address tmp_rtr(router);
+
                 if (wccp2_numrouters > 1) {
-                    /* FIXME INET6 : drop temp conversion */
-                    Ip::Address tmp_rtr(router);
                     comm_udp_sendto(theWccp2Connection,
                                     tmp_rtr,
                                     &wccp_packet,
                                     offset);
                 } else {
-                    send(theWccp2Connection,
-                         &wccp_packet,
-                         offset,
-                         0);
+                    errno = 0;
+                    if (send(theWccp2Connection, &wccp_packet, offset, 0) < offset)
+                        debugs(80, 2, "ERROR: failed to send WCCPv2 HERE_I_AM packet to " << tmp_rtr << " : " << xstrerror());
                 }
             }
+            safe_free(weight);
         }
 
         service_list_ptr = service_list_ptr->next;