]> granicus.if.org Git - procps-ng/commitdiff
pgrep: Do not memleak the contents of proc_t.
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Fri, 18 May 2018 21:32:21 +0000 (07:32 +1000)
memset()ing task and subtask inside their loops prevents free_acquired()
(in readproc() and readtask()) from free()ing their contents (especially
cmdline and environ).

Our solution is not perfect, because we still memleak the very last
cmdline/environ, but select_procs() is called only once, so this is not
as bad as it sounds.

It would be better to leave subtask in its block and call
free_acquired() after the loop, but this function is static (not
exported).

The only other solution is to use freeproc(), but this means replacing
the stack task/subtask with xcalloc()s, thus changing a lot of code in
pgrep.c (to pointer accesses).

Hence this imperfect solution for now.

pgrep.c

diff --git a/pgrep.c b/pgrep.c
index b18df4a748e36f47133fec31a40f88c9c45a7e43..ce8ccae9a18732298289136366995d425cb52d22 100644 (file)
--- a/pgrep.c
+++ b/pgrep.c
@@ -483,6 +483,7 @@ static struct el * select_procs (int *num)
 {
        PROCTAB *ptp;
        proc_t task;
+       proc_t subtask;
        unsigned long long saved_start_time;      /* for new/old support */
        pid_t saved_pid = 0;                      /* for new/old support */
        int matches = 0;
@@ -510,6 +511,7 @@ static struct el * select_procs (int *num)
        }
 
        memset(&task, 0, sizeof (task));
+       memset(&subtask, 0, sizeof (subtask));
        while(readproc(ptp, &task)) {
                int match = 1;
 
@@ -615,8 +617,6 @@ static struct el * select_procs (int *num)
                        // argparse time, but a further
                        // control is free
                        if (opt_threads && !i_am_pkill) {
-                               proc_t subtask;
-                               memset(&subtask, 0, sizeof (subtask));
                                while (readtask(ptp, &task, &subtask)){
                                        // don't add redundand tasks
                                        if (task.XXXID == subtask.XXXID)
@@ -635,19 +635,9 @@ static struct el * select_procs (int *num)
                                        } else {
                                                list[matches++].num = subtask.XXXID;
                                        }
-                                       memset(&subtask, 0, sizeof (subtask));
                                }
                        }
-
-
-
                }
-
-
-
-
-
-               memset (&task, 0, sizeof (task));
        }
        closeproc (ptp);
        *num = matches;