On Sun, Jan 18, 2009 at 02:03:32PM -0500, Dave Johnson wrote:
> It's unclear if ipAddressTable_cache_load is meant to handle IPv6
> addresses and if so, there is clearly brokenness for duplicate IPv6
> link local addresses (which will of course exist when VLANs are
> present)
> 
> _check_entry_for_updates() only does a binary search for the first
> matching ipv4/ipv6 address, ignoring the fact duplicate ip addresses
> appear to be present in the list.  Again, unclear if these duplicates
> should be in the list or not.  someone more familiar with this code
> would have to comment.

Dug around in the upstream bug tracker a little, and found:

[ 1521999 ] ipAddrTable caching code leaks memory
https://sourceforge.net/tracker/index.php?func=detail&aid=1521999&group_id=12694&atid=112694

The patch attached to this upstream bug goes a little too far and
double-frees ipaddress_entry (ipAddressTable_release_rowreq_ctx() frees its
associated data automatically). I've modified it to remove the snmp_log()
noise and double-free; the resulting packaging diff is attached and has not
leaked during the ~half day it's been up here.

The lenny leak must be different, since this fix is already applied upstream
in the lenny snmpd.

john
-- 
John Morrissey          _o            /\         ----  __o
j...@horde.net        _-< \_          /  \       ----  <  \,
www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__
--- net-snmp-5.2.3.orig/debian/patches/50_ipaddrtable_memleak.README
+++ net-snmp-5.2.3/debian/patches/50_ipaddrtable_memleak.README
@@ -0,0 +1 @@
+Fix IP address table memory leak.
--- net-snmp-5.2.3.orig/debian/patches/50_ipaddrtable_memleak.patch
+++ net-snmp-5.2.3/debian/patches/50_ipaddrtable_memleak.patch
@@ -0,0 +1,19 @@
+--- net-snmp-5.3.orig/agent/mibgroup/ip-mib/ipAddressTable/ipAddressTable_data_access.c	2005-12-01 11:00:57.000000000 -0600
++++ net-snmp-5.3/agent/mibgroup/ip-mib/ipAddressTable/ipAddressTable_data_access.c	2006-07-13 10:39:52.816059620 -0500
+@@ -229,9 +229,13 @@ _add_new_entry(netsnmp_ipaddress_entry *
+                                     ipaddress_entry->ia_address_len,
+                                     ipaddress_entry->ia_address,
+                                     ipaddress_entry->ia_address_len))) {
+-        CONTAINER_INSERT(container, rowreq_ctx);
+-        rowreq_ctx->ipAddressLastChanged =
+-            rowreq_ctx->ipAddressCreated = netsnmp_get_agent_uptime();
++        if (CONTAINER_INSERT(container, rowreq_ctx) < 0) {
++            DEBUGMSGTL (("ipAddressTable:access","container insert failed for new entry\n"));
++            ipAddressTable_release_rowreq_ctx(rowreq_ctx);
++            return;
++        }
++        rowreq_ctx->ipAddressLastChanged =
++            rowreq_ctx->ipAddressCreated = netsnmp_get_agent_uptime();
+     } else {
+         if (NULL != rowreq_ctx) {
+             snmp_log(LOG_ERR, "error setting index while loading "

Reply via email to