[prev in list] [next in list] [prev in thread] [next in thread] 

List:       net-snmp-bugs
Subject:    [ net-snmp-Bugs-3560655 ] snmpd 5.6.1.1, memory leak on hpux 11i (all versions)
From:       SourceForge.net <noreply () sourceforge ! net>
Date:       2012-09-17 20:31:58
Message-ID: E1TDhz7-0005C6-Sd () sfs-ml-4 ! v29 ! ch3 ! sourceforge ! com
[Download RAW message or body]

Bugs item #3560655, was opened at 2012-08-22 05:16
Message generated for change (Comment added) made by hardaker
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=3560655&group_id=12694

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: agent
Group: hpux
> Status: Closed
Resolution: Accepted
Priority: 5
Private: No
Submitted By: Jean-Paul VILLETTE (villettejp)
Assigned to: Niels Baggesen (nba)
Summary: snmpd 5.6.1.1, memory leak on hpux 11i (all versions)

Initial Comment:
5.6.1.1 on hpux 11.23, compiled with gcc :

# gcc -v
Using built-in specs.
Target: ia64-hp-hpux11.23
Configured with: /tmp/gcc-4.3.1.tar.gz/gcc-4.3.1/configure --host=ia64-hp-hpux13
Thread model: posix
gcc version 4.3.1 (GCC) 

So it's a 32bit execuatable run on a 64bit OS.

I have found a memory leak in the HPUX specific code in the swun_pstat.c file.

file : agent/mibgroup/host/data_access/swrun_pstat.c

--- net-snmp-5.6.1.1-atsegfault//agent/mibgroup/host/data_access/swrun_pstat.c
Thu Aug  9 12:25:19 2012
***************
*** 52,72 ****
      int                  nproc, i, rc;
      char                *cp1, *cp2;
      netsnmp_swrun_entry *entry;

      pstat_getdynamic( &pst_dyn, sizeof(struct pst_dynamic), 1, 0);
      nproc = pst_dyn.psd_activeprocs;
      proc_table = (struct pst_status *) malloc(nproc*(sizeof(struct pst_status)
));
!     pstat_getproc(proc_table, sizeof(struct pst_status), nproc, 0);

      for ( i=0 ; i<nproc; i++ ) {
!         entry = netsnmp_swrun_entry_create(proc_table[i].pst_pid);
!         if (NULL == entry)
!             continue;   /* error already logged by function */
!         rc = CONTAINER_INSERT(container, entry);

          entry->hrSWRunName_len = snprintf(entry->hrSWRunName,
                                     sizeof(entry->hrSWRunName)-1,
                                            "%s", proc_table[i].pst_ucomm);
          /*
           *  Split pst_cmd into two:
           *     argv[0]   is hrSWRunPath
--- 52,78 ----
      int                  nproc, i, rc;
      char                *cp1, *cp2;
      netsnmp_swrun_entry *entry;
+     int ret=0;

      pstat_getdynamic( &pst_dyn, sizeof(struct pst_dynamic), 1, 0);
      nproc = pst_dyn.psd_activeprocs;
     proc_table = (struct pst_status *) malloc(nproc*(sizeof(struct pst_status)
));
!     ret = pstat_getproc(proc_table, sizeof(struct pst_status), nproc, 0);
!     DEBUGMSGTL(("swrun:load:arch","pstat_getproc return: %d\n",
!                   ret));

      for ( i=0 ; i<nproc; i++ ) {
!        entry = netsnmp_swrun_entry_create(proc_table[i].pst_pid);
!        if (NULL == entry)
!            continue;   /* error already logged by function */
!        rc = CONTAINER_INSERT(container, entry);

+        if (rc != -1) {
          entry->hrSWRunName_len = snprintf(entry->hrSWRunName,
                                     sizeof(entry->hrSWRunName)-1,
                                            "%s", proc_table[i].pst_ucomm);
+         DEBUGMSGTL(("swrun:load:arch"," pst_ucomm : >%s<\n",
+                     proc_table[i].pst_ucomm));
         /*
           *  Split pst_cmd into two:
           *     argv[0]   is hrSWRunPath
***************
*** 106,111 ****
--- 112,120 ----
          entry->hrSWRunPerfMem  = proc_table[i].pst_rssize;
          entry->hrSWRunPerfMem *= getpagesize() / 1024;  /* in kB */
                /* XXX - Check this last calculation */
+        } else {
+         netsnmp_swrun_entry_free( entry );
+        }
      }
      free(proc_table);

The problem occurs when CONTAINER_INSERT() fails inserting a new entry (in my cases \
it was the insertion of ARP table entries and it failed because the entry already \
exists). In this case the memory allocated by the function \
netsnmp_swrun_entry_create() is never freed.

The modification i made to solved this were:
- add a condition to be sure to make the modification in the "entry" only if the \
                insertion worked,
- add a netsnmp_swrun_entry_free (entry) in case of failure in the insertion.

contact me if you need more information...


----------------------------------------------------------------------

> Comment By: Wes Hardaker (hardaker)
Date: 2012-09-17 13:31

Message:
Applied the discussed fix to the 5.5+ branches

----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-09-15 03:16

Message:
what would be the next step now ?

I am sorry i am new to this so do i have to propose a patch ? If yes, a
diff result to set the modifications i did ?



----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-30 01:23

Message:
Exactly, just the #define _PSTAT64 just before the include of sys/pstat.h
 solved two problems liked together :
 - the memory leak i mentionned at first,
 - the garbage displayed when querying hrSWRun.
 
The version of gcc i am using was downloaded from HP as binairies for this
 version of the OS (11.23). There is no HPUX 13 for instant.


----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-30 01:22

Message:
Exactly, just the #define PSTAT64 just before the include of sys/pstat.h
solved two problems liked together :
- the memory leak i mentionned at first,
- the garbage displayed when querying hrSWRun.

The version of gcc i am using was downloaded from HP as binairies for this
version of the OS (11.23). There is no HPUX 13 for instant.


----------------------------------------------------------------------

Comment By: Niels Baggesen (nba)
Date: 2012-08-29 12:35

Message:
So, all that is needed to make it work is the #define _PSTATS64?

You gcc reports
# gcc -v
Using built-in specs.
Target: ia64-hp-hpux11.23
Configured with: /tmp/gcc-4.3.1.tar.gz/gcc-4.3.1/configure
--host=ia64-hp-hpux13

What it don't like is the difference between the target line and the --host
spec. Was that gcc created on the system in question? I know from Solaris,
that if you try to move a binary gcc from one os version another it is sure
to get you into trouble, so I would make sure that it is not such a thing
we see here.


----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-28 01:27

Message:
ok. I did this by changing the source of swrun_pstat.c :

--- swrun_pstat.c       Tue Aug 28 08:31:11 2012
***************
*** 20,25 ****
--- 20,26 ----
  #include <sys/param.h>
  #endif
  #ifdef HAVE_SYS_PSTAT_H
+ #define _PSTAT64
  #include <sys/pstat.h>
  #endif

NOTA : The _LPSTAT64 must be defined before we load pstat.h in order to
define the 64 bit env. for all pstast_xxxxx() functions.
  
It's valid on hpux 11i (all versions, v1 to v3, meaning 11.11 11.23 and
11.31) running a 64 bit system. But not on 32 bit systems.

Just to remember, I am compiling a 32 bit net-snmp package running on a 64
bit system.

I wonder if the best solution should not be to define a more general flag
like __LP64__ when the "configure" detects a 64 bit system.

NOTA : The tests i did after the modification were successfull. Exactly the
same result as the one i did using the modified CFLAGS.

Sorry but i don't understand your remark about gcc and ia64-hp-hpux11.23.



----------------------------------------------------------------------

Comment By: Niels Baggesen (nba)
Date: 2012-08-27 11:52

Message:
My best bet would be to put a #define _PSTATS64 inside swrun_pstat.c

But I am bit worried that your gcc says ia64-hp-hpux13. My experiences with
Solaris says it is a no-no to move a compiled gcc between os releases ...

----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-26 23:45

Message:
First, let's illustrate the garbage i received simply using snmpwalks :

- without _PSTAT64, using net-snmp 5.6.1.1 :

# /opt/iexpress/net-snmp/bin/snmpwalk -c public -v 1 <host>
1.3.6.1.2.1.25.4.2.1.2 
HOST-RESOURCES-MIB::hrSWRunName.0 = STRING: "swapper"
HOST-RESOURCES-MIB::hrSWRunName.1 = ""
HOST-RESOURCES-MIB::hrSWRunName.3 = ""
HOST-RESOURCES-MIB::hrSWRunName.5 = ""
HOST-RESOURCES-MIB::hrSWRunName.6 = ""
HOST-RESOURCES-MIB::hrSWRunName.390003 = STRING: "lan1"
HOST-RESOURCES-MIB::hrSWRunName.622668 = ""
HOST-RESOURCES-MIB::hrSWRunName.655360 = Hex-STRING: 9F FF FF FF 7F 7E 7B
D0 04 
HOST-RESOURCES-MIB::hrSWRunName.40895295 = ""
HOST-RESOURCES-MIB::hrSWRunName.1073741827 = ""
HOST-RESOURCES-MIB::hrSWRunName.1651715922 = STRING:
"s/net-snmp/share/snmp/mibs
/RFC1213-MIB.txt"
HOST-RESOURCES-MIB::hrSWRunName.1818324529 = ""
HOST-RESOURCES-MIB::hrSWRunName.1882156393 = STRING:
"press/net-snmp/share/snmp/
mibs/RFC1213-MIB.txt"
HOST-RESOURCES-MIB::hrSWRunName.1949135726 = STRING:
"/snmp/mibs/RFC1213-MIB.txt
"
HOST-RESOURCES-MIB::hrSWRunName.2138995664 = ""
HOST-RESOURCES-MIB::hrSWRunName.2139001536 = Hex-STRING: C0 03 E7 C0 
HOST-RESOURCES-MIB::hrSWRunName.2684354559 = ""
HOST-RESOURCES-MIB::hrSWRunName.3221481408 = STRING: "@"

Of course, it's exactly the same behavior using hrSWRunID and hrSWRunIndex
for the query.

- with _LPSTAT64 : using net-snmp 5.6.1.1
# /opt/iexpress/net-snmp/bin/snmpwalk -c public -v 1 <host>
1.3.6.1.2.1.25.4.2.1.2 
HOST-RESOURCES-MIB::hrSWRunName.0 = STRING: "swapper"
HOST-RESOURCES-MIB::hrSWRunName.1 = STRING: "init"
HOST-RESOURCES-MIB::hrSWRunName.2 = STRING: "vhand"
HOST-RESOURCES-MIB::hrSWRunName.3 = STRING: "statdaemon"
HOST-RESOURCES-MIB::hrSWRunName.4 = STRING: "unhashdaemon"
HOST-RESOURCES-MIB::hrSWRunName.8 = STRING: "kmemdaemon"
HOST-RESOURCES-MIB::hrSWRunName.9 = STRING: "ioconfigd"
HOST-RESOURCES-MIB::hrSWRunName.10 = STRING: "ObjectThreadPo"
HOST-RESOURCES-MIB::hrSWRunName.11 = STRING: "nfsktcpd"
HOST-RESOURCES-MIB::hrSWRunName.12 = STRING: "autofskd"
HOST-RESOURCES-MIB::hrSWRunName.13 = STRING: "usbhubd"
HOST-RESOURCES-MIB::hrSWRunName.14 = STRING: "lvmkd"
HOST-RESOURCES-MIB::hrSWRunName.15 = STRING: "lvmkd"
....

NOTA : It seems to be the same with the net-snmp version 5.7.X.

NOTA 2: the memory has also disappeared using this solution.




----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-23 06:56

Message:
Sorry wrong cut and paste :

export CC="gcc -g -static-libgcc"
export CFLAGS="-fPIC -D_PSTAT64 "
./configure  --prefix=/opt/iexpress/net-snmp-pstat \
             --disable-debugging \
             --with-defaults \
             --disable-embedded-perl \
             --with-openssl=/opt/openssl \
             --enable-ipv6 \
             --with-transports="UDP UDPIPv6 TCP TCPIPv6" \
             --enable-ucd-snmp-compatibility



----------------------------------------------------------------------

Comment By: Jean-Paul VILLETTE (villettejp)
Date: 2012-08-23 06:55

Message:
You are absolutelly right: no link betweek ARP entries and proc table.

I did what you suggested : have a look at the pstat_getproc(). The actual
return values are garbage. This may explain the source of the memory leak :
the garbage given by the pstat_getproc() created duplicated entries (at
least try to create) in the container which make the insertion to fail,
creating the memory leak.

The pstat_getproc() gives correct values only if the flags _PSTAT64 is
defined during the compilation.

I did create a new version without any modification of the source code just
adding the _PSTAT64 as CFLAGS option:
./configure  --prefix=/opt/iexpress/net-snmp-pstat \
             --disable-debugging \
             --with-defaults \
             --disable-embedded-perl \
             --with-openssl=/opt/openssl \
             --enable-ipv6 \
             --with-transports="UDP UDPIPv6 TCP TCPIPv6" \
             --enable-ucd-snmp-compatibility

I am currently testing this result. Seems to be OK. No more memory leak for
instant.

I have absolutelly no idea where to define this flags in the source code.
Any Idea ?





----------------------------------------------------------------------

Comment By: Niels Baggesen (nba)
Date: 2012-08-22 21:47

Message:
I wonder what ARP entries has to do with proc entries? Duplicates should
not be possible with proc entries identifies by theri PID?
Maybe you could add a check for the return value of pstat_getproc?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=112694&aid=3560655&group_id=12694

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Net-snmp-bugs mailing list
Net-snmp-bugs@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-bugs


[prev in list] [next in list] [prev in thread] [next in thread]