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
[email protected] _-< \_ / \ ---- < \,
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 "