]> granicus.if.org Git - procps-ng/commitdiff
library: address an 'uninitialised value' VALGRIND bug
authorJim Warner <james.warner@comcast.net>
Wed, 7 Sep 2022 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@dropbear.xyz>
Mon, 12 Sep 2022 12:15:28 +0000 (22:15 +1000)
Thanks to valgrind and his --track-origins=yes option,
the problem and solution was suggested as shown below.

[ and it was created in that commit referenced below ]

But, after attacking this problem by adding a memset()
call in pids.c, a 2nd valgrind oops, also shown below,
was encountered. The dynamically acquired 'cmd' again!

[ might help to explain why changes appear excessive ]

Reference(s):
. 1st valgrind discovery
==11111== Conditional jump or move depends on uninitialised value(s)
==11111==    at 0x13425D: stat2proc (readproc.c:582)
==11111==    by 0x137436: look_up_our_self (readproc.c:1613)
==11111==    by 0x132196: fatal_proc_unmounted (pids.c:1388)
==11111==    by 0x11BA4D: before (top.c:3580)
==11111==    by 0x127E10: main (top.c:7173)
==11111==  Uninitialised value was created by a stack allocation
==11111==    at 0x132165: fatal_proc_unmounted (pids.c:1381)

. Jul, 2022 - fatal_proc_unmounted refactored
commit 52bd019d8ca09ecfec34b5020eb7b8d612c315f8

. 2nd valgrind discovery
==22222== 16 bytes in 1 blocks are definitely lost
==22222==    by 0x4A0E60E: strdup (strdup.c:42)
==22222==    by 0x133D00: stat2proc (readproc.c:587)
==22222==    by 0x136E67: look_up_our_self (readproc.c:1613)
==22222==    by 0x131BC7: fatal_proc_unmounted (pids.c:1390)
==22222==    by 0x11B7C6: before (top.c:3580)
==22222==    by 0x127828: main (top.c:7173)

Signed-off-by: Jim Warner <james.warner@comcast.net>
library/include/readproc.h
library/pids.c
library/readproc.c

index d7ff50b374bcf763adf50abd3d33f9f3ca29cfa8..e5fe1e77d696af6336dce006439686bd30611550 100644 (file)
@@ -281,7 +281,7 @@ PROCTAB *openproc(unsigned flags, ... /* pid_t *| uid_t *| dev_t *| char *[, int
 //       with the previous process or thread.
 proc_t *readproc(PROCTAB *__restrict const PT, proc_t *__restrict p);
 proc_t *readeither(PROCTAB *__restrict const PT, proc_t *__restrict x);
-int look_up_our_self(proc_t *p);
+int look_up_our_self(void);
 void closeproc(PROCTAB *PT);
 char **vectorize_this_str(const char *src);
 
index 267f8af3e757d6d4e85734fa90af88a0ae180e20..1a2e8926d4115616578ad22a0d0b2eabf0e1ce79 100644 (file)
@@ -1381,11 +1381,10 @@ PROCPS_EXPORT struct pids_stack *fatal_proc_unmounted (
 {
     struct pids_fetch *fetched;
     unsigned tid;
-    proc_t self;
 
     /* this is very likely the *only* newlib function where the
        context (pids_info) of NULL will ever be permitted */
-    if (!look_up_our_self(&self)
+    if (!look_up_our_self()
     || (!return_self))
         return NULL;
 
index bd71d950f4b4dd31d639bd01ac63b987c0473edf..4bc9d964c93f046d8a179c96fd432d6fb54803b3 100644 (file)
@@ -1602,15 +1602,18 @@ void closeproc(PROCTAB *PT) {
 
 
 //////////////////////////////////////////////////////////////////////////////////
-int look_up_our_self(proc_t *p) {
+int look_up_our_self(void) {
     struct utlbuf_s ub = { NULL, 0 };
     int rc = 0;
+    proc_t p;
 
+    memset(&p, 0, sizeof(proc_t));
     if(file2str("/proc/self", "stat", &ub) == -1){
         fprintf(stderr, "Error, do this: mount -t proc proc /proc\n");
         _exit(47);
     }
-    rc = stat2proc(ub.buf, p);  // parse /proc/self/stat
+    rc = stat2proc(ub.buf, &p); // parse /proc/self/stat
+    free_acquired(&p);
     free(ub.buf);
     return !rc;
 }