Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 180266
Alias:
Product:
Component:
Status: RESOLVED
Resolution: FIXED
Assigned To: Netmon Herd <netmon@gentoo.org>
Hardware:
OS:
Version:
Priority:
Severity:
Reporter: Doug <doug.manley@gmail.com>
Add CC:
CC:
URL:
Summary:
Status Whiteboard:
Keywords:

Filename Description Type Creator Created Size Actions
net-snmp-5.4.1-clientaddr-fix.doug.patch This is my proposed patch. It has the exact same functionality as the current one, but it does not leak. patch Doug 2007-11-20 18:00 0000 2.50 KB Details | Diff
snmpUDPDomain.c This is the patched snmpUDPDomain.c file. text/plain Doug 2007-11-20 18:01 0000 39.76 KB Details
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 180266 depends on: Show dependency tree
Bug 180266 blocks:
Votes: 5    Show votes for this bug    Vote for this bug

Additional Comments: (this is where you put emerge --info)


Not eligible to see or edit group visibility for this bug.






View Bug Activity   |   Format For Printing   |   XML   |   Clone This Bug


Description:   Opened: 2007-05-29 20:06 0000
Hey guys; I wasn't sure whom to contact about this. In the stable 5.4 (tested
on an "emerge-webrsync" on 2007.05.07) release of net-snmp, there is a bug
where some memory from "netsnmp_udp_transport" is not freed.

net-snmp claims to have fixed this bug in a current build. Can the package be
updated or at least marked as unstable?

(Valgrind example):
==23482== 20 bytes in 1 blocks are definitely lost in loss record 5 of 16
==23482== at 0x4A1FB80: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==23482== by 0x4DEFEEA: netsnmp_udp_transport (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DF03E9: netsnmp_udp_create_tstring (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DED1C4: netsnmp_tdomain_transport_full (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DCDFA6: snmp_sess_open (in /usr/lib64/libnetsnmp.so.15.0.0)

From this site:
www.mail-archive.com/net-snmp-coders@lists.sourceforge.net/msg09641.html
----
This is a bug. It is already fixed in revision 15683.

/MF
----

Thanks,
Doug


Reproducible: Always

Steps to Reproduce:
1.Create an SNMP session.
2.Do an snmpget on, say, "sysDescr.0".
3.See what valgrind says.

Actual Results:  

==23482== 20 bytes in 1 blocks are definitely lost in loss record 5 of 16
==23482== at 0x4A1FB80: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==23482== by 0x4DEFEEA: netsnmp_udp_transport (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DF03E9: netsnmp_udp_create_tstring (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DED1C4: netsnmp_tdomain_transport_full (in
/usr/lib64/libnetsnmp.so.15.0.0)
==23482== by 0x4DCDFA6: snmp_sess_open (in /usr/lib64/libnetsnmp.so.15.0.0)

Expected Results:  
==23482== 0 bytes lost

"MF" says that net-snmp has already fixed it:
"This is a bug. It is already fixed in revision 15683."

Is this revision being used by portage?  If not, can it be really soon?

------- Comment #1 From Markus Ullmann 2007-09-06 08:08:10 0000 -------
Please check again with 5.4.1 currently in ~arch

------- Comment #2 From Doug 2007-11-12 20:36:04 0000 -------
(In reply to comment #1)
> Please check again with 5.4.1 currently in ~arch
> 

No luck.  I compiled "net-analyzer/net-snmp-5.4.1-r1" with "~amd64" to no
avail.  It still leaks.


emerge -pv net-snmp
[ebuild   R   ] net-analyzer/net-snmp-5.4.1-r1  USE="diskio ipv6 perl python
ssl tcpd -X -doc -elf -lm_sensors -mfd-rewrites -minimal -rpm (-selinux)
-sendmail -smux" 0 kB


uname -a
Linux peer1-slave 2.6.16-gentoo-r7 #2 SMP Wed Jun 7 02:24:56 UTC 2006 x86_64
AMD Athlon(tm) 64 Processor 3200+ AuthenticAMD GNU/Linux


Here is the valgrind output:
==1924== 462 bytes in 44 blocks are definitely lost in loss record 5 of 6
==1924==    at 0x4A1EDA0: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==1924==    by 0x50E30A1: strdup (in /lib64/libc-2.5.so)
==1924==    by 0x4DFD028: (within /usr/lib64/libnetsnmp.so.15.1.0)
==1924==    by 0x4DFEA33: netsnmp_udp_transport (in
/usr/lib64/libnetsnmp.so.15.1.0)
==1924==    by 0x4DFFA4A: netsnmp_udp_create_tstring (in
/usr/lib64/libnetsnmp.so.15.1.0)
==1924==    by 0x4DF8D09: netsnmp_tdomain_transport_full (in
/usr/lib64/libnetsnmp.so.15.1.0)
==1924==    by 0x4DD2A05: snmp_sess_open (in /usr/lib64/libnetsnmp.so.15.1.0)
==1924==    by 0x4F69194: snmpOpen (in /usr/local/lib64/libsevonebase.so)
==1924==    by 0x4F6938F: snmpOpenByDevId (in
/usr/local/lib64/libsevonebase.so)
==1924==    by 0x401111: nPoll (nPoll.c:118)
==1924==    by 0x402535: main (nPoll.c:739)

------- Comment #3 From Doug 2007-11-13 15:40:06 0000 -------
I did some MD5-checking on the "gz" files from portage and from net-snmp, and
they are identical for "5.4.1".  I built 5.4.1 separately, and it removed the
issue.  I am emerging net-snmp again (I've done this at least three times now)
to see if there's something that I'm missing.

Could there possibly be a USE flag that is the problem?

------- Comment #4 From Doug 2007-11-13 15:49:14 0000 -------
All right.  It seems to work perfectly fine now.  I have no idea what happened
(perhaps I had a total lapse of sanity).

This seems fine.

However, there are some errors:
==25535== ERROR SUMMARY: 5 errors from 3 contexts (suppressed: 5 from 1)
==25535==
==25535== 1 errors in context 1 of 3:
==25535== Syscall param
����������������������������������������
points to uninitialised byte(s)
==25535==    at 0x512C500: sendmsg (in /lib64/libc-2.5.so)
==25535==    by 0x4DF922E: netsnmp_udp_send (snmpUDPDomain.c:180)
==25535==    by 0x4DD197C: snmp_sess_async_send (snmp_api.c:4892)
==25535==    by 0x4DB1349: snmp_sess_synch_response (snmp_client.c:1078)
==25535==    by 0x4F644D4: snmpGetMany (in /usr/local/lib64/libsevonebase.so)
==25535==    by 0x4019D6: nPoll (nPoll.c:426)
==25535==    by 0x402535: main (nPoll.c:740)
==25535==  Address 0x7FEFFDF58 is on thread 1's stack
==25535==
==25535== 2 errors in context 2 of 3:
==25535== Invalid read of size 1
==25535==    at 0x4DE56F9: read_config_files_in_path (read_config.c:1127)
==25535==    by 0x4DE5C86: read_config_files (read_config.c:1199)
==25535==    by 0x4DE5DED: read_configs (read_config.c:864)
==25535==    by 0x4024E1: main (nPoll.c:731)
==25535==  Address 0x5D66292 is 0 bytes after a block of size 74 alloc'd
==25535==    at 0x4A1EDA0: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==25535==    by 0x50DE0A1: strdup (in /lib64/libc-2.5.so)
==25535==    by 0x4DE5649: read_config_files_in_path (read_config.c:1047)
==25535==    by 0x4DE5C86: read_config_files (read_config.c:1199)
==25535==    by 0x4DE5DED: read_configs (read_config.c:864)
==25535==    by 0x4024E1: main (nPoll.c:731)
==25535==
==25535== 2 errors in context 3 of 3:
==25535== Invalid read of size 1
==25535==    at 0x4DE56F9: read_config_files_in_path (read_config.c:1127)
==25535==    by 0x4DE5C86: read_config_files (read_config.c:1199)
==25535==    by 0x4DE5CE4: read_premib_configs (read_config.c:893)
==25535==    by 0x4DC7814: init_snmp (snmp_api.c:836)
==25535==    by 0x4024E1: main (nPoll.c:731)
==25535==  Address 0x5D4151A is 0 bytes after a block of size 74 alloc'd
==25535==    at 0x4A1EDA0: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==25535==    by 0x50DE0A1: strdup (in /lib64/libc-2.5.so)
==25535==    by 0x4DE5649: read_config_files_in_path (read_config.c:1047)
==25535==    by 0x4DE5C86: read_config_files (read_config.c:1199)
==25535==    by 0x4DE5CE4: read_premib_configs (read_config.c:893)
==25535==    by 0x4DC7814: init_snmp (snmp_api.c:836)
==25535==    by 0x4024E1: main (nPoll.c:731)

------- Comment #5 From Doug 2007-11-20 17:24:19 0000 -------
Hi guys.

All right, we did some hardcore tests (finally), and we have the results. 
There IS still a memory leak in net-snmp 5.4.1-r1, and we know where and why
and how.

Basically, the net-snmp 5.4.1 distribution is perfectly fine.  However, the
patch that gentoo installs for the "clientaddr" fix is what creates the leak.

The patch provides some better debug output in one of the internal functions
(among other things).  What it also does is a "strdup" in an "sprintf" command
in the variables section, which is very, very bad (as there is now no pointer
to the address that "strdup" put its output in--so it can't be "free"d).

What's really strange is that there is a comment about "strdup" being very
important.

File: snmpUDPDomain.c
Patch file: net-snmp-5.4.1-clientaddr-fix.patch
Original:
static char *
netsnmp_udp_fmtaddr(netsnmp_transport *t, void *data, int len)
{
    netsnmp_udp_addr_pair *addr_pair = NULL;

    if (data != NULL && len == sizeof(netsnmp_udp_addr_pair)) {
    addr_pair = (netsnmp_udp_addr_pair *) data;
    } else if (t != NULL && t->data != NULL) {
    addr_pair = (netsnmp_udp_addr_pair *) t->data;
    }

    if (addr_pair == NULL) {
        return strdup("UDP: unknown");
    } else {
        struct sockaddr_in *to = NULL;
    char tmp[64];
        to = (struct sockaddr_in *) &(addr_pair->remote_addr);
        if (to == NULL) {
            return strdup("UDP: unknown");
        }

        sprintf(tmp, "UDP: [%s]:%hu",
                inet_ntoa(to->sin_addr), ntohs(to->sin_port));
        return strdup(tmp);
    }
}
Patched:
static char *
netsnmp_udp_fmtaddr(netsnmp_transport *t, void *data, int len)
{
    netsnmp_udp_addr_pair *addr_pair = NULL;

    if (data != NULL && len == sizeof(netsnmp_udp_addr_pair)) {
    addr_pair = (netsnmp_udp_addr_pair *) data;
    } else if (t != NULL && t->data != NULL) {
    addr_pair = (netsnmp_udp_addr_pair *) t->data;
    }

    if (addr_pair == NULL) {
        return strdup("UDP: unknown");
    } else {
        struct sockaddr_in *to = NULL;
    char tmp[64];
        to = (struct sockaddr_in *) &(addr_pair->remote_addr);
        /* Using strdup on the output of inet_ntoa is important! */
        if (to == NULL) {
            sprintf(tmp, "UDP: [%s]->unknown",
                    strdup(inet_ntoa(addr_pair->local_addr)));
        } else {
            sprintf(tmp, "UDP: [%s]->[%s]:%hu",
                    strdup(inet_ntoa(addr_pair->local_addr)),
                    strdup(inet_ntoa(to->sin_addr)), ntohs(to->sin_port)); //<
This is line 113.
        }
        return strdup(tmp);
    }
}

Valgrind output:
==19221== 378 bytes in 36 blocks are definitely lost in loss record 7 of 8
==19221==    at 0x4A1EDA0: malloc (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==19221==    by 0x50E30A1: strdup (in /lib64/libc-2.5.so)
==19221==    by 0x4DFD028: netsnmp_udp_fmtaddr (snmpUDPDomain.c:113)
==19221==    by 0x4DFD5CF: netsnmp_udp_send (snmpUDPDomain.c:263)
==19221==    by 0x4DD239C: snmp_sess_async_send (snmp_api.c:4892)
==19221==    by 0x4DB1D69: snmp_sess_synch_response (snmp_client.c:1078)
==19221==    by 0x4F694D4: snmpGetMany (in /usr/local/lib64/libsevonebase.so)
==19221==    by 0x4019D6: nPoll (nPoll.c:426)
==19221==    by 0x402535: main (nPoll.c:740)

=== Gentoo configuration ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib
--with-install-prefix=/var/tmp/portage/net-analyzer/net-snmp-5.4.1-r1/image/
--with-sys-location=Unknown --with-sys-contact=root@Unknown
--with-default-snmp-version=3 --with-mib-modules='host ucd-snmp/dlmod
ucd-snmp/diskio' --with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
        * Leaks in snmp_sess_synch_response (strdup).

=== Doug 1 (gentoo with debugging) ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib
--with-install-prefix=/var/tmp/portage/net-analyzer/net-snmp-5.4.1-r1/image/
--with-sys-location=Unknown --with-sys-contact=root@Unknown
--with-default-snmp-version=3 --with-mib-modules='host ucd-snmp/dlmod
ucd-snmp/diskio' --with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
--enable-debugging
        * No leaks... (lots of errors).

=== Doug 2 (gentoo without prefix) ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib --with-sys-location=Unknown
--with-sys-contact=root@Unknown --with-default-snmp-version=3
--with-mib-modules='host ucd-snmp/dlmod ucd-snmp/diskio'
--with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
--enable-debugging
        * No leaks... (lots of errors).

=== Doug 2 (gentoo without prefix) ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib --with-sys-location=Unknown
--with-sys-contact=root@Unknown --with-default-snmp-version=3
--with-mib-modules='host ucd-snmp/dlmod ucd-snmp/diskio'
--with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
        * No leaks... (lots of errors).

=== Doug 3 (gentoo without prefix with patch) ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib --with-sys-location=Unknown
--with-sys-contact=root@Unknown --with-default-snmp-version=3
--with-mib-modules='host ucd-snmp/dlmod ucd-snmp/diskio'
--with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
        * Leaks in snmp_sess_synch_response (strdup).

=== Doug 4 (gentoo without prefix with patch with debugging) ===
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
--infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
--localstatedir=/var/lib --with-sys-location=Unknown
--with-sys-contact=root@Unknown --with-default-snmp-version=3
--with-mib-modules='host ucd-snmp/dlmod ucd-snmp/diskio'
--with-logfile=/var/log/net-snmpd.log
--with-persistent-directory=/var/lib/net-snmp --enable-ucd-snmp-compatibility
--enable-shared --enable-as-needed --disable-mfd-rewrites
--enable-embedded-perl --enable-ipv6 --disable-internal-md5 --with-openssl
--with-libwrap --without-rpm --without-bzip2 --without-zlib --without-elf
--with-python-modules --libdir=/usr/lib64 --build=x86_64-pc-linux-gnu
--enable-debugging
        * Leaks in snmp_sess_synch_response (strdup).

------- Comment #6 From Doug 2007-11-20 18:00:22 0000 -------
Created an attachment (id=136511) [details]
This is my proposed patch.  It has the exact same functionality as the current
one, but it does not leak.

This does not do unnecessary "strdup" operations, and it frees the ones that it
does do.

If there is a reason for the "strdup" sprinkling all over the patch, it is not
apparent.  "strdup" should only be necessary for preventing the buffer from
being overwritten by two successive calls in the "sprintf".

------- Comment #7 From Doug 2007-11-20 18:01:06 0000 -------
Created an attachment (id=136513) [details]
This is the patched snmpUDPDomain.c file.

This is the patched snmpUDPDomain.c file.

------- Comment #8 From Peter Volkov 2007-12-27 17:13:42 0000 -------
Should be fixed in net-snmp-5.4.1-r2. I've backport some further changes from
upstream which fixes memory leaks. Please, reopen if it still leaks for you.
Thank you very much for your investigation of the problem.

------- Comment #9 From Doug 2007-12-31 19:31:12 0000 -------
This seems to do the trick; thanks.

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug