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 leak in handle_agentx_packet when using a contextName #58

Closed
jain-mukesh opened this issue Jan 17, 2020 · 4 comments
Closed

Memory leak in handle_agentx_packet when using a contextName #58

jain-mukesh opened this issue Jan 17, 2020 · 4 comments

Comments

@jain-mukesh
Copy link

Our infrastructure code uses net-snmp and I found a memory leak in handle_agentx_packet if contextName is given.

Below is the trace from valgrind

==30360==    at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==30360==    by 0x653F294: snmp_clone_mem (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x653F3E7: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x653F90D: snmp_clone_pdu (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x60E3837: handle_agentx_packet (in /usr/lib64/libnetsnmpagent.so.31.0.2)
==30360==    by 0x6567ED0: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x6568F40: _sess_read (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x65698A8: snmp_sess_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x65698FA: snmp_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x656993B: snmp_read (in /usr/lib64/libnetsnmp.so.31.0.2)
==30360==    by 0x60DF30A: agent_check_and_process (in /usr/lib64/libnetsnmpagent.so.31.0.2)

On further debugging, we found that the internalPDU->contextName was assigned to the internalPDU->community resulting in losing the reference to the assigned memory of internalPDU->contextName. This memory is never free'ed when the internalPDU is deallocated.

Proposed fix:
contextNames memory should be freed before assignment to community, so that the memory is not lost.

In agent/mibgroup/agentx/subagent.c

internal_pdu = snmp_clone_pdu(pdu);

SNMP_FREE(internal_pdu->contextName);

internal_pdu->contextName = (char *) internal_pdu->community;
internal_pdu->contextNameLen = internal_pdu->community_len;

@bvanassche
Copy link
Contributor

I think the above information shows that the Net-SNMP version on your setup is v5.4, v5.5, v5.6 or v5.7. Please retest with the master branch and report whether or not this leak also occurs with the master branch.

@jain-mukesh
Copy link
Author

Thanks @bvanassche . I tested on release 5.8 and 5.3, but will retest this on the master over this week.

jain-mukesh pushed a commit to jain-mukesh/net-snmp that referenced this issue Mar 2, 2020
…for contextName is assigned to community and is hence lost.

This is discussed further in issue net-snmp#58 along with a valgrind report.
@jain-mukesh
Copy link
Author

Hi @bvanassche Sorry, it took me a while to integrate the master code with my internal tools. I confirmed that this leak occurs even with the master branch of net-snmp. My proposed fix is in #78

bvanassche pushed a commit to bvanassche/net-snmp that referenced this issue Mar 9, 2020
Fix a memory leak when contextName is used in the PDU. The memory for
contextName is assigned to community and is hence lost. This patch
fixes the following Valgrind complaint:

at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
by 0x653F294: snmp_clone_mem (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x653F3E7: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x653F90D: snmp_clone_pdu (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x60E3837: handle_agentx_packet (in /usr/lib64/libnetsnmpagent.so.31.0.2)
by 0x6567ED0: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x6568F40: _sess_read (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x65698A8: snmp_sess_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x65698FA: snmp_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x656993B: snmp_read (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x60DF30A: agent_check_and_process (in /usr/lib64/libnetsnmpagent.so.31.0.2)

See also net-snmp#78.
See also net-snmp#58.

[ bvanassche: modified commit message and patch ]
@bvanassche
Copy link
Contributor

Closing this issue since a fix has been applied.

LeSpocky pushed a commit to LeSpocky/net-snmp that referenced this issue Apr 13, 2020
Fix a memory leak when contextName is used in the PDU. The memory for
contextName is assigned to community and is hence lost. This patch
fixes the following Valgrind complaint:

at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
by 0x653F294: snmp_clone_mem (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x653F3E7: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x653F90D: snmp_clone_pdu (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x60E3837: handle_agentx_packet (in /usr/lib64/libnetsnmpagent.so.31.0.2)
by 0x6567ED0: ??? (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x6568F40: _sess_read (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x65698A8: snmp_sess_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x65698FA: snmp_read2 (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x656993B: snmp_read (in /usr/lib64/libnetsnmp.so.31.0.2)
by 0x60DF30A: agent_check_and_process (in /usr/lib64/libnetsnmpagent.so.31.0.2)

See also net-snmp#78.
See also net-snmp#58.

[ bvanassche: modified commit message and patch ]
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

2 participants