Bug 684031

Summary: bugs in procps-3.2.7-ps-cgroup.patch and procps-3.2.8-pmap-smaps.patch
Product: Red Hat Enterprise Linux 6 Reporter: Kamil Dudka <kdudka>
Component: procpsAssignee: Jan Görig <jgorig>
Status: CLOSED ERRATA QA Contact: BaseOS QE - Apps <qe-baseos-apps>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.1CC: albert, bnater, ovasik, syeghiay
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: procps-3.2.8-17.el6 Doc Type: Bug Fix
Doc Text:
There have been found various memory leaks and incorrect use of close function in procps. These leaks have been fixed.
Story Points: ---
Clone Of:
: 690078 (view as bug list) Environment:
Last Closed: 2011-05-19 14:08:17 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 690078    

Description Kamil Dudka 2011-03-10 22:40:56 UTC
Description of problem:
- procps-3.2.7-ps-cgroup.patch passes the pointer returned by fopen() to close() as a file descriptor, which can never work

- procps-3.2.8-pmap-smaps.patch introduces a call of fopen() which is never closed

- procps-3.2.8-pmap-smaps.patch uses value of uninitialized variable 'start' in 
one_proc()


Version-Release number of selected component (if applicable):
procps-3.2.8-16.el6

Comment 4 Jan Görig 2011-03-17 14:05:13 UTC
Uninitialized variable 'start' isn't problem because of /proc/*/smaps file syntax.

Other problems are fixed in procps-3.2.8-17.el6.

Comment 5 Kamil Dudka 2011-03-17 14:29:14 UTC
(In reply to comment #4)
> Uninitialized variable 'start' isn't problem because of /proc/*/smaps file
> syntax.

It's still worth nullifying the variable.  As you can see, the return value of sscanf() is also not checked a few lines later, so it would most likely crash on any deviation from the expected input data format.

Comment 6 Kamil Dudka 2011-03-17 17:05:56 UTC
Please consider also the following patch for the next iteration:

--- a/ps/output.c
+++ b/ps/output.c
@@ -1187,10 +1187,10 @@ static int pr_context(char *restrict const outbuf, const proc_t *restrict const
   if(!ps_getpidcon && !tried_load){
     void *handle = dlopen("libselinux.so.1", RTLD_NOW);
     if(handle){
-      dlerror();
       ps_getpidcon = dlsym(handle, "getpidcon");
       if(dlerror())
         ps_getpidcon = 0;
+      dlclose(handle);
     }
     tried_load++;
   }

Comment 8 Kamil Dudka 2011-03-17 17:24:55 UTC
(In reply to comment #6)
>    if(!ps_getpidcon && !tried_load){
>      void *handle = dlopen("libselinux.so.1", RTLD_NOW);
>      if(handle){
> -      dlerror();
>        ps_getpidcon = dlsym(handle, "getpidcon");
>        if(dlerror())
>          ps_getpidcon = 0;
> +      dlclose(handle);

This is actually a completely wrong fix, I'll try to prepare a better one...

Comment 9 Kamil Dudka 2011-03-17 17:35:27 UTC
Maybe better to not fix the dlopen() handle leakage in el6.  The ps_getpidcon variable is declared static and reused on the subsequent calls of pr_context().  A proper fix would require to make the handle variable globally visible and call dlclose() somewhere else.

Comment 12 Jan Görig 2011-03-23 09:43:23 UTC
The procps-3.2.8-pmap-smaps.patch should be rewritten but it is too late to do this in 6.1. Cloning to 6.2.

Comment 14 Jan Görig 2011-03-23 10:10:59 UTC
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
There have been found various memory leaks and incorrect use of close function in procps.
These leaks has been fixed.

Comment 16 Jan Görig 2011-03-23 10:31:30 UTC
    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1,2 +1,2 @@
 There have been found various memory leaks and incorrect use of close function in procps.
-These leaks has been fixed.+These leaks have been fixed.

Comment 18 errata-xmlrpc 2011-05-19 14:08:17 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0708.html