From 046883b9d382f18fcdeacfe4b66693b1c41f9e5d Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Wed, 7 Sep 2022 00:00:00 -0500 Subject: [PATCH] library: address an 'uninitialised value' VALGRIND bug 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 --- library/include/readproc.h | 2 +- library/pids.c | 3 +-- library/readproc.c | 7 +++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/library/include/readproc.h b/library/include/readproc.h index d7ff50b3..e5fe1e77 100644 --- a/library/include/readproc.h +++ b/library/include/readproc.h @@ -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); diff --git a/library/pids.c b/library/pids.c index 267f8af3..1a2e8926 100644 --- a/library/pids.c +++ b/library/pids.c @@ -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; diff --git a/library/readproc.c b/library/readproc.c index bd71d950..4bc9d964 100644 --- a/library/readproc.c +++ b/library/readproc.c @@ -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; }