Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MEMORY LEAKS in net-snmp 5.9 #300

Closed
boggarapupriyanka opened this issue Jun 8, 2021 · 12 comments
Closed

MEMORY LEAKS in net-snmp 5.9 #300

boggarapupriyanka opened this issue Jun 8, 2021 · 12 comments

Comments

@boggarapupriyanka
Copy link

boggarapupriyanka commented Jun 8, 2021

I ran valgrind over night on the package net-snmp 5.9 with v1/v2c configuration . Could see more memory leaks in the package.

==10448== HEAP SUMMARY:
==10448==     in use at exit: 306,589 bytes in 1,353 blocks
==10448==   total heap usage: 181,444,190 allocs, 181,442,837 frees, 307,792,996,550 bytes allocated
==10448==
==10448== 8 bytes in 1 blocks are definitely lost in loss record 10 of 320
==10448==    at 0x403106C: calloc (vg_replace_malloc.c:711)
==10448==    by 0x427D992: ??? (in /lib/libnetsnmp.so.35.0.1)
==10448==    by 0x427A283: CONTAINER_INSERT_HELPER (in /lib/libnetsnmp.so.35.0.1)
==10448==    by 0x427A300: CONTAINER_INSERT (in /lib/libnetsnmp.so.35.0.1)
==10448==    by 0x40FAB90: ??? (in /lib/libnetsnmpmibs.so.35.0.1)
==10448==    by 0x427BB36: ??? (in /lib/libnetsnmp.so.35.0.1)
==10448==    by 0x40FAF26: ifTable_container_load (in /lib/libnetsnmpmibs.so.35.0.1)
==10448==    by 0x40F72E3: ??? (in /lib/libnetsnmpmibs.so.35.0.1)
==10448==    by 0x404B2D9: ??? (in /lib/libnetsnmpagent.so.35.0.1)
==10448==    by 0x42601D4: run_alarms (in /lib/libnetsnmp.so.35.0.1)
==10448==    by 0x804B641: ??? (in /bin/snmpd)
==10448==    by 0x804ADA9: ??? (in /bin/snmpd)
==10448== 
==10448== 286 bytes in 2 blocks are definitely lost in loss record 209 of 320
==10448==    at 0x402F5ED: malloc (vg_replace_malloc.c:299)
==10448==    by 0x49BB965: strdup (in /lib/libc-2.27.so)
==10448==    by 0x4267A6B: netsnmp_parse_args (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x4068F50: snmpd_parse_config_trapsess (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x426DED3: run_config_handler (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x426F2AD: read_config (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x426F35D: read_config_with_type_when (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x426F6B5: read_configs_optional (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x4270052: read_configs (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x425F273: init_snmp (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x804A9B6: ??? (in /bin/snmpd)
==10448==    by 0x4959E50: (below main) (in /lib/libc-2.27.so)
==10448==
==10448== 68 bytes in 1 blocks are possibly lost in loss record 158 of 326
==10448==    at 0x403106C: calloc (vg_replace_malloc.c:711)
==10448==    by 0x414FBB5: netsnmp_access_tcpconn_entry_create (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x4150052: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x4150A95: netsnmp_arch_tcpconn_container_load (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x414FA44: netsnmp_access_tcpconn_container_load (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x4155C38: tcpListenerTable_container_load (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x415465D: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x404B756: ??? (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x404CA5A: netsnmp_cache_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x4053425: table_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==
==10448== 140 bytes in 1 blocks are possibly lost in loss record 204 of 326
==10448==    at 0x403106C: calloc (vg_replace_malloc.c:711)
==10448==    by 0x41556E9: tcpListenerTable_allocate_rowreq_ctx (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x4155928: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x429D19E: ??? (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x4155CCC: tcpListenerTable_container_load (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x415465D: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x404B756: ??? (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x404CA5A: netsnmp_cache_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x4053425: table_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405EA6A: netsnmp_call_handlers (in /lib/libnetsnmpagent.so.40.0.0)

==10448== 2,352 (588 direct, 1,764 indirect) bytes in 1 blocks are definitely lost in loss record 308 of 326
==10448==    at 0x403106C: calloc (vg_replace_malloc.c:711)
==10448==    by 0x40E1DB7: mteTrigger_run (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x427E030: run_alarms (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x804B877: ??? (in /bin/snmpd)
==10448==    by 0x804AED9: ??? (in /bin/snmpd)
==10448==    by 0x4959E50: (below main) (in /lib/libc-2.27.so)
==10448==
==10448== 4,704 bytes in 8 blocks are possibly lost in loss record 319 of 326
==10448==    at 0x402F5ED: malloc (vg_replace_malloc.c:299)
==10448==    by 0x4245EF9: ??? (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x4248091: netsnmp_query_walk (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x40E1DFC: mteTrigger_run (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x427E030: run_alarms (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x804B877: ??? (in /bin/snmpd)
==10448==    by 0x804AED9: ??? (in /bin/snmpd)
==10448==    by 0x4959E50: (below main) (in /lib/libc-2.27.so)
==10448==
==10448== 94,988 (63,980 direct, 31,008 indirect) bytes in 457 blocks are definitely lost in loss record 326 of 326
==10448==    at 0x403106C: calloc (vg_replace_malloc.c:711)
==10448==    by 0x41556E9: tcpListenerTable_allocate_rowreq_ctx (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x4155928: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x429D19E: ??? (in /lib/libnetsnmp.so.40.0.0)
==10448==    by 0x4155CCC: tcpListenerTable_container_load (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x415465D: ??? (in /lib/libnetsnmpmibs.so.40.0.0)
==10448==    by 0x404B756: ??? (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x404CA5A: netsnmp_cache_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x4053425: table_helper_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405E78D: netsnmp_call_handler (in /lib/libnetsnmpagent.so.40.0.0)
==10448==    by 0x405EA6A: netsnmp_call_handlers (in /lib/libnetsnmpagent.so.40.0.0)
==10448==
==10448== LEAK SUMMARY:
==10448==    definitely lost: 64,568 bytes in 458 blocks
==10448==    indirectly lost: 32,772 bytes in 459 blocks
==10448==      possibly lost: 4,912 bytes in 10 blocks
==10448==    still reachable: 204,337 bytes in 426 blocks
==10448==         suppressed: 0 bytes in 0 blocks
==10448== Reachable blocks (those to which a pointer was found) are not shown.
==10448== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==10448==
==10448== For counts of detected and suppressed errors, rerun with: -v
==10448== Use --track-origins=yes to see where uninitialised values come from
==10448== ERROR SUMMARY: 57673 errors from 52 contexts (suppressed: 0 from 0)

@bvanassche @hardaker Please have a look and provide quick resolution

Please let us know if anything is required from our end.

@fenner
Copy link
Member

fenner commented Jun 8, 2021

Can you provide your snmpd.conf? The mteTrigger leak can only happen with certain configuration, so it would be helpful to know what specific configuration you used.

@boggarapupriyanka
Copy link
Author

Thanks for responding @fenner

Please find the snmpd.conf below
Note: we are encrypting the community strings for security purpose.

SF01V_SO01_SFOS 19.0.0 EAP0-Build3733# cat /cfs/system/snmpd.conf 
agentAddress udp:161,udp6:161
sysName test
syslocation Bangalore
sysContact Priyanka
sysDescr 
sysObjectID 1.3.6.1.4.1.2604.5
rocommunity '$sfos$7$0$6ICK7l0zEMDv6R8mQ0pvCDZXDSQS_bFT9r-1NsPncoOZU0oivd9_bOeGdeiO_lbDtJ3GDQ5KnR8Saj9oJp8IQQ~~-VRhCW0IafhmFspWaZMJQyGTJrTv9S4YWECFqZnx6xY~' 192.168.56.20
rocommunity6 '$sfos$7$0$6ICK7l0zEMDv6R8mQ0pvCDZXDSQS_bFT9r-1NsPncoOZU0oivd9_bOeGdeiO_lbDtJ3GDQ5KnR8Saj9oJp8IQQ~~-VRhCW0IafhmFspWaZMJQyGTJrTv9S4YWECFqZnx6xY~' 192.168.56.20
authtrapenable 1
trapsess -v1 192.168.56.20 -c '$sfos$7$0$6ICK7l0zEMDv6R8mQ0pvCDZXDSQS_bFT9r-1NsPncoOZU0oivd9_bOeGdeiO_lbDtJ3GDQ5KnR8Saj9oJp8IQQ~~-VRhCW0IafhmFspWaZMJQyGTJrTv9S4YWECFqZnx6xY~'
trapsess -v2c 192.168.56.20 -c '$sfos$7$0$6ICK7l0zEMDv6R8mQ0pvCDZXDSQS_bFT9r-1NsPncoOZU0oivd9_bOeGdeiO_lbDtJ3GDQ5KnR8Saj9oJp8IQQ~~-VRhCW0IafhmFspWaZMJQyGTJrTv9S4YWECFqZnx6xY~'
notificationEvent  linkUpTrap '.1.3.6.1.6.3.1.1.5.4' '.1.3.6.1.2.1.2.2.1.1' '.1.3.6.1.2.1.2.2.1.2' '.1.3.6.1.2.1.2.2.1.7' '.1.3.6.1.2.1.2.2.1.8'
notificationEvent  linkDownTrap '.1.3.6.1.6.3.1.1.5.3' '.1.3.6.1.2.1.2.2.1.1' '.1.3.6.1.2.1.2.2.1.2' '.1.3.6.1.2.1.2.2.1.7' '.1.3.6.1.2.1.2.2.1.8'
CreateUser '__internal_v3_trap_user'
rouser '__internal_v3_trap_user'
monitor  -u '__internal_v3_trap_user' -r 5 -e linkUpTrap 'Generate linkUp' '.1.3.6.1.2.1.2.2.1.8' != 2
monitor  -u '__internal_v3_trap_user' -r 5 -e linkDownTrap 'Generate linkDown' '.1.3.6.1.2.1.2.2.1.8' == 2 

@boggarapupriyanka boggarapupriyanka changed the title memory leaks in net-snmp 5.9 MEMORY LEAKS in net-snmp 5.9 Jun 8, 2021
@boggarapupriyanka
Copy link
Author

boggarapupriyanka commented Jun 12, 2021

I fixed few leaks with the code change below. Actually the entries which are failed to insert into the container are not getting freed which causes memory leak in tcpListenerTable.

--- a/agent/mibgroup/tcp-mib/tcpListenerTable/tcpListenerTable_data_access.c
+++ b/agent/mibgroup/tcp-mib/tcpListenerTable/tcpListenerTable_data_access.c
@@ -184,7 +184,9 @@ _add_connection(netsnmp_tcpconn_entry *e
                                                      (char *) entry->loc_addr,
                                                      entry->loc_addr_len,
                                                      entry->loc_port))) {
-        CONTAINER_INSERT(container, rowreq_ctx);
+        if ((CONTAINER_INSERT(container, rowreq_ctx)) < 0) {
+            tcpListenerTable_release_rowreq_ctx(rowreq_ctx);
+        }
     } else {
         if (rowreq_ctx) {
             snmp_log(LOG_ERR, "error setting index while loading "
--- a/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c
+++ b/agent/mibgroup/tcp-mib/data_access/tcpConn_linux.c
@@ -242,7 +242,9 @@ _load4(netsnmp_container *container, u_i
          * add entry to container
          */
         entry->arbitrary_index = CONTAINER_SIZE(container) + 1;
-        CONTAINER_INSERT(container, entry);
+        if ((CONTAINER_INSERT(container, entry)) < 0) {
+            netsnmp_access_tcpconn_entry_free(entry);
+        }
     }
 
     fclose(in);
@@ -389,7 +391,9 @@ _load6(netsnmp_container *container, u_i
          * add entry to container
          */
         entry->arbitrary_index = CONTAINER_SIZE(container) + 1;
-        CONTAINER_INSERT(container, entry);
+        if ((CONTAINER_INSERT(container, entry)) < 0) {
+            netsnmp_access_tcpconn_entry_free(entry);
+        }
     }
 
     fclose(in);

@fenner @bvanassche It would be very helpful if we can get any update on mteTrigger leak.

@boggarapupriyanka
Copy link
Author

boggarapupriyanka commented Jun 22, 2021

Hi @bvanassche @fenner
Any update on this?

@bvanassche
Copy link
Contributor

Has it already been tried to run snmpd under Valgrind and with leak checking enabled (e.g. valgrind --trace-children=yes --trace-children-skip=/usr/bin/env,/bin/sed,/bin/ls,/bin/sh,/bin/bash,/usr/bin/sed,/usr/bin/ls,/usr/bin/make,/usr/bin/perl --track-origins=no --leak-check=full --show-leak-kinds=definite --num-callers=32)?

bvanassche added a commit that referenced this issue Jun 22, 2021
@boggarapupriyanka
Copy link
Author

boggarapupriyanka commented Jun 23, 2021

Thanks for the response @bvanassche

I am seeing the leaks in netsnmp module when i run valgrind using below one.

valgrind --tool=memcheck --leak-check=full --track-fds=yes --log-file=/tmp/snmpd_valgrind.log snmpd -f -A -c /cfs/system/snmpd.conf --logTimestamp=true &

And i gave a try with the valgrind arguments you mentioned, could see leaks in mtetrigger_run, netsnmp_query_walk and CONTAINER_INSERT_HELPER ( please refer for the o/p #300 (comment))

I have fixed leak in netsnmp_parse_args with the below code:

--- a/snmplib/snmp_parse_args.c
+++ b/snmplib/snmp_parse_args.c
@@ -436,7 +436,7 @@ netsnmp_parse_args(int argc,
                     goto out;
 		}
 	    } else {
-		Cpsz = strdup(optarg);
+		Cpsz = optarg;
 	    }
             break;

I tried debugging the mteTrigger leaks which i mentioned in the forum but unfortunately I didn't get any clue. Could you please check why the leaks are happening?

@bvanassche
Copy link
Contributor

The above change looks wrong to me. Please provide the leak complaint reported by Valgrind.

@boggarapupriyanka
Copy link
Author

Please find the valgrind log file

snmpd_valgrind_8514.log

bvanassche added a commit to bvanassche/net-snmp that referenced this issue Jun 27, 2021
This patch fixes the following Valgrind complaint:

14 bytes in 2 blocks are definitely lost in loss record 20 of 322
   at 0x402F5ED: malloc (vg_replace_malloc.c:299)
   by 0x4548965: strdup (in /lib/libc-2.27.so)
   by 0x424D370: netsnmp_parse_args (in /lib/libnetsnmp.so.35.0.1)
   by 0x4064519: snmpd_parse_config_trapsess (in /lib/libnetsnmpagent.so.35.0.1)
   by 0x425287E: run_config_handler (in /lib/libnetsnmp.so.35.0.1)
   by 0x42539DB: read_config (in /lib/libnetsnmp.so.35.0.1)
   by 0x4253A5F: read_config_with_type_when (in /lib/libnetsnmp.so.35.0.1)
   by 0x4253D67: read_configs_optional (in /lib/libnetsnmp.so.35.0.1)
   by 0x4254642: read_configs (in /lib/libnetsnmp.so.35.0.1)
   by 0x4245849: init_snmp (in /lib/libnetsnmp.so.35.0.1)
   by 0x804A887: ??? (in /bin/snmpd)
   by 0x44E6E50: (below main) (in /lib/libc-2.27.so)

See also net-snmp#300
bvanassche added a commit to bvanassche/net-snmp that referenced this issue Jun 28, 2021
This patch fixes the following Valgrind complaint:

14 bytes in 2 blocks are definitely lost in loss record 20 of 322
   at 0x402F5ED: malloc (vg_replace_malloc.c:299)
   by 0x4548965: strdup (in /lib/libc-2.27.so)
   by 0x424D370: netsnmp_parse_args (in /lib/libnetsnmp.so.35.0.1)
   by 0x4064519: snmpd_parse_config_trapsess (in /lib/libnetsnmpagent.so.35.0.1)
   by 0x425287E: run_config_handler (in /lib/libnetsnmp.so.35.0.1)
   by 0x42539DB: read_config (in /lib/libnetsnmp.so.35.0.1)
   by 0x4253A5F: read_config_with_type_when (in /lib/libnetsnmp.so.35.0.1)
   by 0x4253D67: read_configs_optional (in /lib/libnetsnmp.so.35.0.1)
   by 0x4254642: read_configs (in /lib/libnetsnmp.so.35.0.1)
   by 0x4245849: init_snmp (in /lib/libnetsnmp.so.35.0.1)
   by 0x804A887: ??? (in /bin/snmpd)
   by 0x44E6E50: (below main) (in /lib/libc-2.27.so)

See also net-snmp#300 .

Fixes: 09f2cba ("libsnmp, netsnmp_parse_args(): Fix memory leaks in error paths")
@bvanassche
Copy link
Contributor

Commit afc1854 should fix the community string leak. Please retest.

fenner pushed a commit to fenner/net-snmp that referenced this issue Jun 30, 2021
@boggarapupriyanka
Copy link
Author

Commit afc1854 should fix the community string leak. Please retest.

I tested trap changes. Looks fine i don't see leak in community string.

@bvanassche
Copy link
Contributor

Please reopen if this needs further attention.

@krishnacx
Copy link

@boggarapupriyanka , @fenner
I am using net-snmp-5.9.1. Any major memory leak fixes that need to be patched in my workspace. Your inputs would be highly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants