Haven't seen any response to my earlier mail to this list, so I just thought I'd throw my query in again now when there's a lot more activity than when I originally posted it.

Since it's related to memory leaks in a quite unusual scenario I'm guessing that not that many have encountered it, but any memory leak I'd say is a bad memory leak (besides that we in our usage encounter this not too rarely).

The suggested way of fixing the leak has been working just fine on our end since end of may. We applied the fix to the 5.7.2 source release, but the patch provided was against git head as of 2013-05-30 (latest change was 2013-05-15, 47abcb995d10e05e94420da9177193f3c3ebc7c0). Also attaching a new patch based on current head (d738284e30c26edf318eaaffe022e7e143c0ad36).

I'm hoping for any comments/feedback on whether this is a horrible way of doing things or if its an acceptable solution to the memory leak.

Best regards
Patrik Thunström / patrik.thunst...@basset.com

On 2013-05-30 16:47, Patrik Thunström wrote:
Hi!

First mail to this mailing list, and I admit I haven't managed to read up on the last few years of discussion, so sorry if I'm bringing up already discussed topics.

I've recently taken over maintenance of a few of the modules we're using for both ends of our SNMP functionality, where we have a standard snmpd acting as AgentX with multiple SNMPv3 agents, each with their own context. On the other end of things we boil our SNMP requests down to a simple web interface where each of these contexts, which are entered by the users, are shown as separate sub trees/application nodes. The users may enter any context name, which we will gladly try to find.

This leads us to the issue; If the agents with that specific context has not yet connected to the snmpd, we get the expected behavior, protocol/response wise, as this case is detected in snmp_agent.c:1933 (in the 5.7.2 source code), and quite well documented. However, "on the way out", nothing is done with the pdu->securityStateRef in this case, which in our case generates a leak of about 1GB of unfreed securityStateRefs if I leave it overnight (with 65 unknown contexts for each of which there is a request every 5 seconds).

In the "correct case" where the context exists, the securityStateRef is kept, as it's queued up in netsnmp_handle_request, and later on freed once the request has been fulfilled.

If an agent with that specific context has been connected once, the context is no longer handled as unknown and everything seems to be handled fine, and memory is freed.

Our invocation of handle_snmp_packet is from snmp_api.c:5411 (sp->callback being executed).

The solution I have in place right now, which seems to work fine, is to just after the call to netsnmp_remove_and_free_agent_snmp_session add the same routines as is available at snmp_api.c:5260 and snmp_api.c:5422 (which I can see in the git repository has been converted to the function free_securityStateRef).

This does eliminate the memory leak on our test environment at least.

I thought I'd throw this in for discussion if you believe it is a bad approach to solving the memory leak for this case? I've also attached a unified diff against current git HEAD, to get a easier grasp on the fix...

One thought I had myself is that it might be a cleaner approach to handle the error inside of snmp_api.c instead, to leave the agent code less dependant on the pdu memory operations, but the unhandled context logic seems to be the most direct application of a fix, so I'm a bit split about whats the best approach, and me just starting to look at the code base gives me just about zero knowledge about the convention used for this kind of fixes in this library. :)

As for our environment, and versions we noticed this issue with: Both win32 and win64 msvc2010 builds of the snmpd application shows the leak, the issue was noticed in the 5.6.1 version of Net-SNMP, and the issue still persisting in the 5.7.2 source code. Servers this leak has been seen on is Windows Server 2003, 2008, 2008R2 and 2012.

Best regards
Patrik Thunström / patrik.thunst...@basset.com

 agent/snmp_agent.c         | 1 +
 include/net-snmp/pdu_api.h | 3 +++
 snmplib/snmp_api.c         | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/agent/snmp_agent.c b/agent/snmp_agent.c
index 51eb287..f724911 100644
--- a/agent/snmp_agent.c
+++ b/agent/snmp_agent.c
@@ -1965,6 +1965,7 @@ handle_snmp_packet(int op, netsnmp_session * session, int 
reqid,
              * drop the request 
              */
             netsnmp_remove_and_free_agent_snmp_session(asp);
+            free_securityStateRef(pdu);
             return 0;
         } else {
             /*
diff --git a/include/net-snmp/pdu_api.h b/include/net-snmp/pdu_api.h
index 125595d..d77ea46 100644
--- a/include/net-snmp/pdu_api.h
+++ b/include/net-snmp/pdu_api.h
@@ -20,6 +20,9 @@ netsnmp_pdu    *snmp_fix_pdu(  netsnmp_pdu *pdu, int idx);
 NETSNMP_IMPORT
 void            snmp_free_pdu( netsnmp_pdu *pdu);
 
+NETSNMP_IMPORT
+void            free_securityStateRef(netsnmp_pdu* pdu);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/snmplib/snmp_api.c b/snmplib/snmp_api.c
index 297f1f7..afee15c 100644
--- a/snmplib/snmp_api.c
+++ b/snmplib/snmp_api.c
@@ -3832,7 +3832,7 @@ snmpv3_parse(netsnmp_pdu *pdu,
     return SNMPERR_SUCCESS;
 }                               /* end snmpv3_parse() */
 
-static void
+void
 free_securityStateRef(netsnmp_pdu* pdu)
 {
     struct snmp_secmod_def *sptr = find_sec_mod(pdu->securityModel);
------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

Reply via email to