Bug 684031 - bugs in procps-3.2.7-ps-cgroup.patch and procps-3.2.8-pmap-smaps.patch
Summary: bugs in procps-3.2.7-ps-cgroup.patch and procps-3.2.8-pmap-smaps.patch
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: procps
Version: 6.1
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: rc
: ---
Assignee: Jan Görig
QA Contact: BaseOS QE - Apps
URL:
Whiteboard:
Depends On:
Blocks: 690078
TreeView+ depends on / blocked
 
Reported: 2011-03-10 22:40 UTC by Kamil Dudka
Modified: 2011-05-19 14:08 UTC (History)
4 users (show)

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.
Clone Of:
: 690078 (view as bug list)
Environment:
Last Closed: 2011-05-19 14:08:17 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:0708 0 normal SHIPPED_LIVE procps bug fix update 2011-05-18 18:10:00 UTC

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


Note You need to log in before you can comment on or make changes to this bug.