From da06b8fa5974ff8d651e45e55d52475c371054fe Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Tue, 28 Oct 2014 00:00:00 -0500 Subject: [PATCH] top: tweak forest view protections for forking anomaly A recent commit eliminated the potential for a storage violation with forest view mode. It occurred when some program (erroneously?) created a lengthy forking loop. However, the associated commit message was misleading. The message implied that an unexpected order following a sort on start_time was the cause of storage overruns and a 'char' used to track nesting level only distorts the display when it goes negative. Actually, the truth is really just the opposite. Any start_time sort quirk causes no harm while that 'char' can yield corruption. Should some child end up sorted ahead of its parent by way of an extremely unlikely shared start_time the end result is such a child will be displayed unnested just like init or kthreadd along with all its own children. However, if nesting levels exceeded 255 (and became 0) a massive array overrun could be triggered when such a task and *all* its children were added to an array for the second time. Exactly how much storage was violated depended on the number of children that zeroed process had spawned (hinted at via either SIGSEGV or SIGABRT). The earlier commit limited nested levels to 100 so the root cause of the storage violation was already fixed. The potential for distorted nesting levels due to sort on start_time would seem to remain. But it's extremely unlikely that 2 tasks would share the same start_time. Even so, a new #define has been introduced which makes top impervious to the order of tasks such that a qsort is no longer necessary (providing an init/systemd task exists & was harvested as the first task by readproc). It can be utilized if distorted nesting ever becomes a real issue. But since there is a 5-10% performance hit with that, we'll continue using start_time as default. References(s): commit ce70017eb1927be51f73cbe0a0b4babcc502607e Signed-off-by: Jim Warner --- top/top.c | 26 ++++++++++++++++++-------- top/top.h | 3 +++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/top/top.c b/top/top.c index 5126d464..ce635081 100644 --- a/top/top.c +++ b/top/top.c @@ -4906,18 +4906,25 @@ static int Tree_idx; // frame_make resets to zero 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 - || (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? + if (Tree_idx < Frame_maxtask) { // immunize against insanity + 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 +#ifdef TREE_SCANALL + for (i = 0; i < Frame_maxtask; i++) { + if (i == self) continue; +#else + for (i = self + 1; i < Frame_maxtask; i++) { +#endif + 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)) + forest_adds(i, level + 1); // got one child any others? + } } } // end: forest_adds +#ifndef TREE_SCANALL /* * Our qsort callback to order a ppt by the non-display start_time * which will make us immune from any pid, ppid or tgid anomalies @@ -4927,6 +4934,7 @@ static int forest_based (const proc_t **x, const proc_t **y) { if ( (*x)->start_time < (*y)->start_time ) return -1; return 0; } // end: forest_based +#endif /* @@ -4944,7 +4952,9 @@ static void forest_create (WIN_t *q) { hwmsav = Frame_maxtask; Tree_ppt = alloc_r(Tree_ppt, sizeof(proc_t*) * hwmsav); } +#ifndef TREE_SCANALL qsort(Seed_ppt, Frame_maxtask, sizeof(proc_t*), (QFP_t)forest_based); +#endif for (i = 0; i < Frame_maxtask; i++) // avoid any hidepid distortions if (!Seed_ppt[i]->pad_3) // identify real or pretend trees forest_adds(i, 1); // add as parent plus its children diff --git a/top/top.h b/top/top.h index 5c94da32..1b1b1a18 100644 --- a/top/top.h +++ b/top/top.h @@ -59,6 +59,7 @@ //#define STRINGCASENO /* case insenstive compare/locate versions */ //#define TERMIOS_ONLY /* just limp along with native input only */ //#define TREE_NORESET /* sort keys do NOT force forest view OFF */ +//#define TREE_SCANALL /* rescan array w/ forest view, avoid sort */ //#define USE_X_COLHDR /* emphasize header vs. whole col, for 'x' */ //#define VALIDATE_NLS /* validate the integrity of all nls tbls */ @@ -781,7 +782,9 @@ typedef struct WIN_t { //atic void keys_xtra (int ch); /*------ Forest View support -------------------------------------------*/ //atic void forest_adds (const int self, int level); +#ifndef TREE_SCANALL //atic int forest_based (const proc_t **x, const proc_t **y); +#endif //atic void forest_create (WIN_t *q); //atic inline const char *forest_display (const WIN_t *q, const proc_t *p); /*------ Main Screen routines ------------------------------------------*/