From ce70017eb1927be51f73cbe0a0b4babcc502607e Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Thu, 23 Oct 2014 00:00:00 -0500 Subject: [PATCH] top: provide some protection against forking anomalies This commit will eliminate a very nasty bug associated with top's forest view mode. It addresses a potential SIGSEGV/SIGABRT that was only encountered when another program (erroneously?) creates a lengthy forking loop. If the growing list of nested children is sufficiently fast such that proc_t start_time is duplicated between children then the sort upon which top relies might not produce the expected order. That, in turn, could cause the forest_adds function to initially miss some child. But that missed child would be caught by forest_create and eventually would cause our array boundary overrun. Such overrun occurs when some child of that originally *missed* child is found and a duplicate add attempted. In correcting this bug we'll also use this opportunity to prohibit a borrowed proc_t padding byte (char) from going negative. If the nesting level exceeded 127, the effect was an "unnesting" with the snprintf width then viewed as flag+width also yielding left justification. Henceforth, we'll limit nesting to 100 with subsequent children shown as " + ", not the usual " `- " prefix. References(s): https://bugzilla.redhat.com/show_bug.cgi?id=1153642 http://www.freelists.org/post/procps/Bug-in-the-forrest-view,6 Signed-off-by: Jim Warner --- top/top.c | 15 +++++++++------ top/top.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/top/top.c b/top/top.c index 985a8bc8..5126d464 100644 --- a/top/top.c +++ b/top/top.c @@ -4895,22 +4895,24 @@ static void keys_xtra (int ch) { * ( plus, maintain alphabetical order with carefully chosen ) * ( function names: forest_a, forest_b, forest_c & forest_d ) * ( each with exactly one letter more than its predecessor! ) */ -static proc_t **Seed_ppt; // temporary window ppt ptr -static proc_t **Tree_ppt; // resized by forest_create -static int Tree_idx; // frame_make initializes +static proc_t **Seed_ppt; // temporary win ppt pointer +static proc_t **Tree_ppt; // forest_create will resize +static int Tree_idx; // frame_make resets to zero /* * This little recursive guy is the real forest view workhorse. * He fills in the Tree_ppt array and also sets the child indent * level which is stored in an unused proc_t padding byte. */ -static void forest_adds (const int self, const int level) { +static void forest_adds (const int self, int level) { int i; + if (level > 100) level = 101; // our arbitrary nests limit Tree_ppt[Tree_idx] = Seed_ppt[self]; // add this as root or child Tree_ppt[Tree_idx++]->pad_3 = level; // borrow 1 byte, 127 levels for (i = self + 1; i < Frame_maxtask; i++) { - if (Seed_ppt[self]->tid == Seed_ppt[i]->tgid + if ((Seed_ppt[self]->tid == Seed_ppt[i]->tgid || (Seed_ppt[self]->tid == Seed_ppt[i]->ppid && Seed_ppt[i]->tid == Seed_ppt[i]->tgid)) + && Tree_idx < Frame_maxtask) // shouldn't happen, but has forest_adds(i, level + 1); // got one child any others? } } // end: forest_adds @@ -4963,7 +4965,8 @@ static inline const char *forest_display (const WIN_t *q, const proc_t *p) { const char *which = (CHKw(q, Show_CMDLIN)) ? *p->cmdline : p->cmd; if (!CHKw(q, Show_FOREST) || 1 == p->pad_3) return which; - snprintf(buf, sizeof(buf), "%*s%s", 4 * (p->pad_3 - 1), " `- ", which); + if (p->pad_3 > 100) snprintf(buf, sizeof(buf), "%400s%s", " + ", which); + else snprintf(buf, sizeof(buf), "%*s%s", 4 * (p->pad_3 - 1), " `- ", which); return buf; } // end: forest_display diff --git a/top/top.h b/top/top.h index 8f61d82f..5c94da32 100644 --- a/top/top.h +++ b/top/top.h @@ -780,7 +780,7 @@ typedef struct WIN_t { //atic void keys_window (int ch); //atic void keys_xtra (int ch); /*------ Forest View support -------------------------------------------*/ -//atic void forest_adds (const int self, const int level); +//atic void forest_adds (const int self, int level); //atic int forest_based (const proc_t **x, const proc_t **y); //atic void forest_create (WIN_t *q); //atic inline const char *forest_display (const WIN_t *q, const proc_t *p);