Commit Graph

497 Commits

Author SHA1 Message Date
Jim Warner
86611b461f top: the '#define SCROLLVAR_NO' is bent but not broken
This patch simply avoids an 'unused' variable warning.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-09-12 21:27:46 +10:00
Jim Warner
5e1c339bef top: speed up the collapsed children forest view logic
In forest view mode, once a collapsible parent process
and all of its children (if any) have been identified,
there is no longer a need to scan the remaining tasks.

So this patch will just force a new scan for any other
'Hide_pid' entries which might remain to be identified
after a targeted parent has been completely processed.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-08-13 20:50:01 +10:00
Jim Warner
cfe5b38a19 top: miscellaneous accumulated tweaks to code/comments
This patch includes the following miscellaneous stuff:

. ensure 1 space before any '*' ptr sizeof() reference

. explain the rather cryptic 'ioa' guy a little better

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-08-13 20:50:01 +10:00
Jim Warner
f635d8a369 top: existing 'Inspect' pipe feature now more flexible
Currently, it isn't possible to establish an 'Inspect'
pipe that relies on SIGINT to end. That's because this
signal will also end the parent process (top) as well.

So this patch will temporarily ignore that signal when
processing any 'Inspect' pipe, allowing one like this:

. pipe ^I Trace Calls ^I /usr/bin/strace -r -p %d 2>&1

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-08-01 21:27:44 +10:00
Jim Warner
1dd11af4c1 top: avoid unrecognized 'Inspect' entries memory leaks
Upon startup there exists the potential for some minor
memory leakage should some rcfile 'Inspect' entries be
invalid. By delaying any malloc/strdup until after the
entries are completely validated we will prevent that.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-08-01 21:27:44 +10:00
Jim Warner
4a44f9e3fa top: ensure collapsed children cpu reported accurately
Parent tasks with collapsed children should have their
cpu reflect any unseen tasks only under the following:

1) When built without TREE_VCPUOFF having been defined

2) Exclusively when 'Show_FOREST' display mode was set

3) And only under the current window when in alternate
display mode (except if TREE_VWINALL has been defined)

So, this commit just ensures these objectives are met.

Reference(s):
. issue that began odyssey
https://gitlab.com/procps-ng/procps/issues/99
. original cpu implementation
commit 3da7318683

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-08-01 21:27:44 +10:00
Jim Warner
786bffc253 top: fix the collapsed children cpu segmentation fault
While that 'Hide_cpu' value will always be zero unless
there are collapsed children, the damn array will only
be present when a window's in 'Show_FOREST' view mode.

Reference(s):
https://www.freelists.org/post/procps/important-improvements-to-top,8

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-07-18 20:17:22 +10:00
Jim Warner
8626141d8f top: '^V' is now 'v' (collapse/expand children toggle)
Using Ctrl-V for the collapse children key now appears
as a mistake. First, it's too close to that Ctrl-C key
which would prematurely terminate top. Second, a lower
case 'v' was unused and perfectly compliments an upper
case 'V' which is used to toggle 'forest view' itself.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/99

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-07-18 20:17:22 +10:00
Jim Warner
8c9daf45ab top: some minor renaming, reformatting and rearranging
This commit just addresses these miscellaneous issues:
. always use 'p' for pointers to that proc_t structure
. always match order of local #undef to parent #define
. forest_create use of array index made more efficient

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-07-18 20:17:22 +10:00
Jim Warner
38a668909f top: parent total cpu includes collapsed children, doc
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-07-17 21:00:51 +10:00
Jim Warner
3da7318683 top: parent total cpu includes collapsed children, pgm
Now, when a parent's children have been collapsed, the
cpu used by those unseen tasks will disappear no more.
Instead such tics will be added to the parent's total.

[ if one wished a return to the 'land of lost tics', ]
[ the '#define TREE_VCPUOFF' directive is available. ]

------------------------------------------------------
Note: With collapsible parents now displaying children
cpu usage, it will eventually be noticed the cpu stats
for the summary area and task areas often vary widely.

It's worth a reminder that for top's summary area each
individual cpu and the cpu summary is limited to 100%,
regardless of how many tics a linux kernel may export.

An individual task is limited to 100% times the number
of threads. But, in no case will cpu usage ever exceed
100% times total number of processors. Such limits are
further reduced under 'Solaris' mode ('I' toggle off).
In this mode, a task cpu usage will never exceed 100%.
These limits will now also apply to collapsed parents.

In addition to those influences, results are subjected
to kernel timer sampling anomalies and the distortions
inherent in a small sample size, made worse by smaller
delay intervals. Often there is just 1 or 2 tics for a
few tasks at smaller intervals such as: 1/10th second.

Anyway, should questions on this subject arise, a good
starting point, beyond the reminders above, is the 1st
link listed below. Those other links were derivatives.

Reference(s):
. from the kernel documentation
https://www.kernel.org/doc/Documentation/cpu-load.txt
. as mentioned in the above kernel documentation
https://lkml.org/lkml/2007/2/12/6
. from above, with many more links on the subject
https://www.boblycat.org/~malc/apc/

Signed-off-by: Jim Warner <james.warner@comcast.net>

top: parent total cpu includes collapsed children, pgm
2018-07-17 21:00:51 +10:00
Jim Warner
e66c36794a top: a refactor to prepare for including collapsed cpu
So that the impact (minimal) of the next commit can be
isolated, this commit just involves a little renaming,
reformat plus a refactor of some proc_t pointer logic.

[ renaming, relocation and changes to 'user_matched' ]
[ wasn't strictly necessary, but now mirrors newlib. ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-07-17 21:00:51 +10:00
Jim Warner
c6e68e2fed top: a few tweaks for those scrolling (mostly) changes
This patch just addresses some edge cases with respect
to 'unseen' tasks. Given the ability to preserve other
filters in the rcfile, it's entirely possible the very
first task(s) may not be visible at top startup. Also,
when switching between windows ('a'/'w') we should try
to always position its row #1 on some visible process.

Lastly, a window might have *NO* visible tasks at all.
Therefore, protect 'window_hlp' from an infinite loop.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-26 21:22:12 +10:00
Jim Warner
95df5a06f4 top: normalize vertical scrolling for hidden processes
To my knowledge, nobody has ever complained about some
anomalies when scrolling vertically if tasks should be
hidden from view. This can happen with the user filter
('u/U') or other filter ('o/O') features. And although
some tasks are not shown, they still impact scrolling.

This is most apparent when that scroll coordinates msg
is on ('C') & up/down arrow keys used (vs. pgup/pgdn).

Now that we can collapse/expand forked children, there
is a potential for yet more of those hidden processes.

So this commit normalizes vertical scrolling providing
an expected behavior. In other words, the up/down keys
skip the unseen tasks to reposition on a visible task.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:57 +10:00
Jim Warner
78dcaffad7 top: refactor the 'scroll coordinates' message support
This patch is simply preparation for upcoming vertical
scrolling enhancements. With those changes, it will be
impossible to predict what the beginning task position
should be at the time the message is currently issued.

This patch will allow such a message to be shown after
the individual windows' tasks have all been displayed.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:57 +10:00
Jim Warner
3943b185a9 top: allow collapsible forest view children, documents
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:57 +10:00
Jim Warner
14eb7275d9 top: allow collapsible forest view children, pgm logic
The issue cited below really dealt with preserving the
'Other filter' criteria in the rcfile. But as an aside
the htop 'F6' feature (collapsed children) was raised.

I took that as an implied challenge and decided to try
implementing a similar feature in top. So, this commit
will now provide a brand new forest view toggle ('^V')
which will be used to collapse/expand forked children.

[ this patch will also lead to additional patches in ]
[ support of more rational vertical scrolling, since ]
[ many more tasks might now be hidden in some window ]

Reference(s):
. where this secondary issue was raised
https://gitlab.com/procps-ng/procps/issues/99

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
57f0ff367c top: to prepare for collapse, move forest view support
In anticipation of a new collapsible child feature, we
will have to make some forest view variables available
to that 'keys_task()' function. This commit just moves
the forest view logic ahead of tertiary input support.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
06d4d985a6 top: 'other filters' saved with config file, documents
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
a6f86732f2 top: 'other filters' saved with config file, pgm logic
Well, after the rearranging and refactoring, all those
active 'other filter' entries for each window will now
be preserved in the user's configuration file via 'W'.

For raising the issue below, thanks to Marco Ippolito.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/99

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
993d22211f top: refactored some more peripheral 'inspect' support
These modifications are being made now in anticipation
of some coming 'other filter' config file changes. Our
entries must be written last to the rc file since that
is where the users have been told to 'echo' additions.

Therefore, that 'config_insp' function must be adapted
to anticipate a passed buffer that was already primed.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
80057ec06f top: refactored some of that 'other filtering' support
If we are to support preserving 'other filter' entries
in the rcfile, then the current logic setting up those
osel entries for a WIN_t must be shareable for startup
and when interacting with a user. So, this commit just
repositions this current code in a shareable function.

[ along the way, we give the prior guy a proper name ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
1fe5a5415f top: reposition some of that 'other filtering' support
When we get around to saving that 'Other Filter' stuff
in the rcfile, we'll need access to the Fieldstab plus
the justify_pad() function. So this commit repositions
two 'osel' functions in anticipation of adding 1 more.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
2b7779fdeb top: refactor some of that configuration files support
The 'config_file()' function was getting a little long
in the tooth, so this commit simply renames/rearranges
some stuff anticipating 'other filters' in the rcfile.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
e691f75b81 top: let's honor <Esc> key on color mapping screen too
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
291d98ee50 top: make rcfile duplicate fields check more efficient
Jeeze, there was no need to employ *both* strchr() and
strrchr() when ensuring fields hadn't been duplicated.

So let's avoid one of those function calls completely.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-23 22:03:56 +10:00
Jim Warner
949a3c9c2e top: a tweak to the recent 256-color termninal support
We now use the actual terminfo 'max_colors' value with
the 'color mapping' screen, not that hard coded '256'.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/96
. introduced 256 color support
commit cf057d2fe5

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-13 22:11:29 +10:00
Jim Warner
9ea85bc653 top: treat all of those vertical scroll keys uniformly
When not displaying all tasks (the 'i' toggle is off),
the concept of vertical scrolling has no real meaning.

However, only 2 keys (up/down) impacting that vertical
position were currently being disabled with this mode.

This patch will extend such treatment to the following
additional vertical impact keys: pgup,pgdn,home & end.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-13 22:11:28 +10:00
Jim Warner
9dd7a97c4a top: force return to row 1 for thread mode transitions
This program does a good job of policing that vertical
scrolled position, ensuring that total tasks are never
exceeded. However, during transitions from thread mode
to normal task mode (the 'H' toggle) that wasn't true.

And while there was no real harm done, it did make the
use of up/down arrow keys "appear" disabled especially
if that scroll message was not displayed ('C' toggle).

This patch simply forces a return to row #1 whenever a
user toggles that display between thread & task modes.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-13 22:11:28 +10:00
Jim Warner
42f0a341ba top: fix 'iokey()' flaw preventing proper translations
As it turns out, the very first entry in the 'iokey()'
tinfo_tab was preventing the proper translation of the
simulated PgUp/PgDn keys (ctrl+meta+k/j). Ignoring the
tortured history behind the most recent change to that
entry, this patch restores the previous value and once
again properly translates these particular keystrokes.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-13 22:11:28 +10:00
Jim Warner
cf057d2fe5 top: provide the means to exploit a 256-color terminal
With the Qualys security audit, we began to harden our
treatment of the top rcfile. In particular, the values
read were checked so as to prevent some malicious user
from editing it in order to achieve an evil objective.

However when it came to colors I was surprised to find
that at least one user edited the rcfile for 256-color
support. Unfortunately, our new checks prevented this.

So this commit will provide the means to exploit those
extra colors with no need to manually edit the rcfile.

Reference(s):
https://gitlab.com/procps-ng/procps/issues/96

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-06-07 21:35:59 +10:00
Jim Warner
fe23360520 top: sanitized some potentially corrupt 'Inspect' data
This guards against rcfile 'Inspect' entries which may
include non-printable characters. While this shouldn't
occur, we have no real control over those crazy users.

[ and, while such data can't be used maliciously, it ]
[ does adversely impact such a user's screen display ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-31 20:28:32 +10:00
Jim Warner
434530a038 top: prevent buffer overruns in 'inspection_utility()'
When a Qualys patch was reverted as being unwarranted,
1 specific problem their patch had, in fact, prevented
was re-introduced. This patch corrects that oversight.

Reference(s):
. qualys patch revert
commit c502678715

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-31 20:28:32 +10:00
Jim Warner
a42742b0df top: add another field sanity check in 'config_file()'
Until the Qualys security audit I had never considered
it a possibility that some malicious person might edit
the top config file to achieve some nefarious results.

And while the Qualys approach tended to concentrate on
the symptoms from such an effort, subsequent revisions
more properly concentrated on startup and that rcfile.

This commit completes those efforts with 1 more field.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-31 20:28:32 +10:00
Jim Warner
cc5c9e6c1e top: don't mess with groff line length in man document
I've long since forgotten why the attempt to influence
groff line lengths was made. However, I did receive an
email regarding problems formatting postscript output.

Hopefully this patch will eliminate any such problems.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-31 20:28:32 +10:00
Jim Warner
c699f2e26d top: just respond to the increased command name length
The command name for running tasks is displayed by top
in a variable length field, so the increase from 16 to
64 bytes was not a problem. However, there's one place
where top is sensitive to length - insp_view_choice().

So, this patch just bumps a buffer used to display it.

Reference(s):
. increased 'comm' length
commit 2cfdbbe897

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:48:13 +10:00
Jim Warner
607edc2064 top: eliminate a couple of warnings of -Wunused-result
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:47:58 +10:00
Jim Warner
de807a639f top: ensure sane rcfile values for the remaining stuff
This will protect some remaining rcfile variables from
a possible manual editing of top's configuration file.

[ and correct two #error related boo-boos introduced ]
[ with the system default rcfile in the commit shown ]

Reference(s):
. introduced /etc/topdefaultrc
commit 3e6a208ae5

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:47:02 +10:00
Jim Warner
129b7e2b44 top: Prevent out-of-bounds writes in PUFF(). __Tweaked
This commit moves some overhead to the Batch mode path
where it's needed. And given the new 'else if' test we
can delete some now redundant logic in the other path.

Reference(s):
. original qualys patch
0117-top-Prevent-out-of-bounds-writes-in-PUFF.patch
commit 059ae8b512

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:46:07 +10:00
Jim Warner
d34c6eb5b4 top: address 'show_special()' o-o-b read/write concern
This patch addresses a potential (but unlikely) buffer
overflow by reducing, if necessary, a memcpy length by
3 bytes to provide for an eol '\0' and 2 unused buffer
positions which also might receive the '\0' character.

[ note to future analysis tool: just because you see ]
[ binary data being manipulated in the routine, that ]
[ doesn't mean such function was passed binary data! ]

Reference(s):
. original qualys patch
0116-top-Fix-out-of-bounds-read-write-in-show_special.patch
commit ed8f6d9cc6

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:45:36 +10:00
Jim Warner
2fb269a5b0 top: Fix out-of-bounds read/write in show_... REVERTED
I'm reverting this patch to prepare for some alternate
solution. In that solution I will address point #1 but
point #2 is based on a wrong assumption. There will be
no binary data ever found in the 'glob' passed to this
show_special() function. It is now always simple text.

------------------------------------------------ original commit message
This patch fixes two problems:

1/ In the switch case 0, if sub_end is at the very end of lin[], the two
null-byte writes are off-by-two (a stack-based buffer overflow). Replace
this end-of-string "emulation" with an equivalent test on ch (and then
goto/break out of the loop).

2/ "sub_end += 2" jumps over the null-byte terminator in lin[] if the
line contains a raw (without a tilde) \001-\010 character. Detect such a
null-byte terminator and goto/break out of the loop.

Note: in the case of a raw \001-\010 character, the character at
"sub_end + 1" is never processed (it is skipped/jumped over); this is
not a security problem anymore (since 2/ was fixed), so we decided not
to change this behavior, for backward-compatibility.
------------------------------------------------------------------------

Reference(s):
. original qualys patch
0116-top-Fix-out-of-bounds-read-write-in-show_special.patch
commit ed8f6d9cc6

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:44:35 +10:00
Jim Warner
fdb58974e2 top: prevent buffer overflow potential in all routines
Whereas that original patch (since reversed) addressed
some symptoms related to manually edited config files,
this solution deals with root causes. And it goes much
beyond any single top field by protecting all of top's
fields. Henceforth, a duplicated field is not allowed.

Reference(s):
. original qualys patch
0114-top-Prevent-buffer-overflow-in-calibrate_fields.patch
commit c424a64331

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:44:11 +10:00
Jim Warner
72ddc1c33d top: Prevent buffer overflow in calibrate_... REVERTED
Here, again, we have an example of attacking a problem
by addressing the symptoms. And that assertion made in
the original commit message is true if only if someone
had manually (maliciously) edited the top config file.

So let's reverse the original patch & thus prepare for
a proper solution addressing the cause, not a symptom.

Reference(s):
. original qualys patch
0114-top-Prevent-buffer-overflow-in-calibrate_fields.patch
commit c424a64331

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:43:08 +10:00
Jim Warner
c502678715 top: Protect scat() from buffer overflows. ___REVERTED
The whole idea was to make top's 'scat' function small
and very quick, unlike that standard 'strcat' routine.

To achieve that end we ignore the potential for buffer
overruns and trust callers to provide adequate dest's.

Reference(s):
. original qualys patch
0109-top-Protect-scat-from-buffer-overflows.patch
commit 9c745975b2

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:42:36 +10:00
Jim Warner
59f39da852 top: concede integer overflow risks in procs_refresh()
This is as far as we need go with respect to the issue
of integer overflow addressed in that reference below.

That patch, of course, was reversed to prepare for us.

Reference(s):
. original qualys patch
0105-top-Prevent-integer-overflows-in-procs_refresh.patch
commit 131e5e2fe6

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:38:19 +10:00
Jim Warner
c9dfcdebdc top: Prevent integer overflows in procs_re... REVERTED
That patch referenced below is being reverted because:

. By design, no other top macro looks like a function.
Instead, they all contain some minimal capitalization.
The 'grow_by_size' macro stands out like a sore thumb.

. We would need to approach 400+ million tasks for for
the 1st addressed problem to produce integer overflow.

. And a 2nd check against SSIZE_MAX remains a mystery.

Me thinks a system on which top is running will suffer
ENOMEM before we need to worry about integer overflow.

Reference(s):
. original qualys patch
0105-top-Prevent-integer-overflows-in-procs_refresh.patch
commit 131e5e2fe6

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:37:29 +10:00
Jim Warner
3b2235c266 top: check sortindx risk exposure (not treat symptoms)
Rather than validate the window's 'sortindx' each time
it was referenced (as was done in the patch below), we
now ensure the validity just once when the config file
is read. Thereafter, a running top will police bounds.

Reference(s):
. original qualys patch
0102-top-Check-sortindx.patch
commit d5b8ac7139

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:24:36 +10:00
Jim Warner
fb8cee585f top: Check sortindx. _________________________REVERTED
Here's yet another example of dealing with a potential
problem at the symptom level, instead of addressing it
at the source. So, we will reverse that original patch
referenced below in preparation for a proper solution.

[ at the least, this ugly code should have used that ]
[ existing MAXTBL macro, making it a little prettier ]

Reference(s):
. original qualys patch
0102-top-Check-sortindx.patch
commit d5b8ac7139

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:23:57 +10:00
Jim Warner
53e2654726 top: other graph_cpus, graph_mems, and summ_mscale fix
This patch replaces an original patch referenced below
which has now been reversed. We now validate variables
'graph_cpus', 'graph_mems' and 'summ_mscale' just once
at startup. Thereafter, top enforces the proper range.

[ we afford the same treatment to that 'task_mscale' ]
[ variable, which was ignored in the original patch. ]

Reference(s):
. original qualys patch
0099-top-Check-graph_cpus-graph_mems-and-summ_mscale.patch
commit cd8ba5670e

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:22:56 +10:00
Jim Warner
d1fbc5fbd4 top: Check graph_cpus, graph_mems, and sum... REVERTED
The variables graph_cpus, graph_mems & summ_mscale are
all well managed in a running top. They were, however,
each vulnerable to tampering via the rcfile. So rather
than continually addressing the symptoms, we'll attack
the root cause just once at startup in the next patch.

Reference(s):
. original qualys patch
0099-top-Check-graph_cpus-graph_mems-and-summ_mscale.patch
commit cd8ba5670e

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:19:35 +10:00
Jim Warner
e531c78140 top: Do not default to the cwd in configs_r... Tweaked
While it's only documented (so far) in commit text and
an occasional email I've tried to maintain some coding
standards primarily for reference/navigation purposes.
They also served, I felt, as useful mental challenges.

Someday I will get around to formerly documenting them
but in the meantime here are the ones for this commit:

. functions are grouped into logical (i hope) sections
. functions & sections are ordered to avoid prototypes
. function names are alphabetical within every section
. all functions & sections must be referenced in top.h

This patch just attempts to honor the above standards,
while also covering this new behavior in the man page.

[ please note that the net result of these 2 patches ]
[ is simply to avoid pathname truncations should our ]
[ limit of 1024 be exceeded. they do not have a role ]
[ in solving the 'local privilege escalation' issue. ]

[ and we can never prevent a user from setting their ]
[ HOME var to a directory writable by some attacker! ]

[ the only real protection for that CVE-2018-1122 is ]
[ those soon to be enhanced rcfile integrity checks, ]
[ achieved through several of the following patches. ]

Reference(s):
. original qualys patch
0097-top-Do-not-default-to-the-cwd-in-configs_read.patch
commit b45c4803dd

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-05-19 21:17:59 +10:00
Qualys Security Advisory
059ae8b512 top: Prevent out-of-bounds writes in PUFF().
This patch prevents three problems:

1/ Because snprintf() returns "the number of characters (excluding the
terminating null byte) which would have been written to the final string
if enough space had been available", _eol may point past the end of _str
and write out-of-bounds (in Batch mode).

2/ _eol is never checked against _str, so "while (*(--_eol) == ' ');"
may point _eol below _str and write out-of-bounds (in Batch mode).

3/ Sanity-check Pseudo_row to protect the strcpy().
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
ed8f6d9cc6 top: Fix out-of-bounds read/write in show_special().
This patch fixes two problems:

1/ In the switch case 0, if sub_end is at the very end of lin[], the two
null-byte writes are off-by-two (a stack-based buffer overflow). Replace
this end-of-string "emulation" with an equivalent test on ch (and then
goto/break out of the loop).

2/ "sub_end += 2" jumps over the null-byte terminator in lin[] if the
line contains a raw (without a tilde) \001-\010 character. Detect such a
null-byte terminator and goto/break out of the loop.

Note: in the case of a raw \001-\010 character, the character at
"sub_end + 1" is never processed (it is skipped/jumped over); this is
not a security problem anymore (since 2/ was fixed), so we decided not
to change this behavior, for backward-compatibility.
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
6b8b102cf2 top: Harden calibrate_fields().
- Make sure i is at least 1 before "i - 1" and "--i".

- Initialize endpflg (to 0, as it was originally, since it is static)
  before the "for" loop (the "break" may leave endpflg uninitialized,
  for example).
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
c424a64331 top: Prevent buffer overflow in calibrate_fields().
pflgsall[] can contain PFLAGSSIZ = 100 elements, each iteration of the
loop can write 3 elements to pflgsall[], and there are EU_MAXPFLGS = 58
iterations: a buffer overflow (it can be triggered via the configuration
file, for example, by filling "fieldscur" with the "sortindx" flag).
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
a71ac048e6 top: Impose a minimum on Screen_cols.
The safety of the critical function task_show() depends on the sanity of
Screen_cols. Just copy the tests on w_cols to Screen_cols (from the same
function adj_geometry()).
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
b4d068e624 top: Prevent integer overflow in adj_geometry(). 2018-05-19 07:33:15 +10:00
Qualys Security Advisory
23baeb1175 top: Limit Width_mode to SCREENMAX.
adj_geometry() limits to SCREENMAX too, but belt and suspenders, and
might as well tell the user about it.
2018-05-19 07:33:15 +10:00
Qualys Security Advisory
8ab8c1a469 top: Prevent integer overflows in config_file() and other_selection(). 2018-05-19 07:33:15 +10:00
Qualys Security Advisory
9c745975b2 top: Protect scat() from buffer overflows.
Several of these buffer overflows can actually be triggered (through the
configuration file for example): in config_file(), inspection_utility(),
and show_special().
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
0847390b83 top: Always exit from sig_abexit().
The default action for SIGURG is to ignore the signal, for example.
This is very similar to the patch "ps/display.c: Always exit from
signal_handler()."
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
7fc7062127 top: Initialize struct sigaction in before(). 2018-05-19 07:32:34 +10:00
Qualys Security Advisory
d69966962c top: Fix snprintf() call in capsmk().
Replace "snprintf(msg, sizeof(pmt)" with "snprintf(msg, sizeof(msg)".
Luckily sizeof(pmt) == sizeof(msg), but fix it anyway.
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
131e5e2fe6 top: Prevent integer overflows in procs_refresh().
This is very similar to our patch against integer overflows in
readproctab*().
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
b6be15d3cb top: Initialize cp in task_show().
Found no problematic case at the moment, but this is a cheap
just-in-case.
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
713381b10d top: Protect macro parameters. 2018-05-19 07:32:34 +10:00
Qualys Security Advisory
d5b8ac7139 top: Check sortindx.
Every time sortindx is used as an index, or loaded from the
configuration file. Otherwise it leads to out-of-bounds reads and
arbitrary code execution.
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
bbe58d7e0a top: Check width and col.
Otherwise they may lead to out-of-bounds writes (snprintf() returns the
number of characters which would have been written if enough space had
been available).

Also, make sure buf is null-terminated after COLPLUSCH has been written.
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
97a989cbcd top: Check Rc.fixed_widest.
Otherwise it leads to crashes (for example, setting it to 2147483600 in
the configuration file segfaults top).
2018-05-19 07:32:34 +10:00
Qualys Security Advisory
cd8ba5670e top: Check graph_cpus, graph_mems, and summ_mscale.
Otherwise they lead to out-of-bounds reads and format-string bugs.

Since these variables are set/written to in several places (for example,
config_file()), check them in the only place where they are read/used.

Also, constify the static gtab[]s.
2018-05-19 07:32:22 +10:00
Qualys Security Advisory
bd91bbf7f1 top: Check i when setting Curwin in config_file().
Otherwise it leads to out-of-bounds reads (and maybe writes).
2018-05-19 07:32:22 +10:00
Qualys Security Advisory
b45c4803dd top: Do not default to the cwd in configs_read().
If the HOME environment variable is not set, or not absolute, use the
home directory returned by getpwuid(getuid()), if set and absolute
(instead of the cwd "."); otherwise, set p_home to NULL.

To keep the changes to a minimum, we rely on POSIX, which requires that
fopen() fails with ENOENT if the pathname (Rc_name) is an empty string.
This integrates well into the existing code, and makes write_rcfile()
work without a change.

Also, it makes the code in configs_read() easier to follow: only set and
use p_home if safe, and only set Rc_name if safe (in all the other cases
it is the empty string, and the fopen() calls fail). Plus, check for
snprintf() truncation (and if it happens, reset Rc_name to the empty
string).

Important note: top.1 should probably be updated, since it mentions the
fallback to the current working directory.
2018-05-19 07:32:22 +10:00
Qualys Security Advisory
e877d4f4c4 top: Fix double-fclose() in configs_read().
It happens only if RCFILE_NOERR is defined (it is not, by default).
2018-05-19 07:32:22 +10:00
Jim Warner
029a463172 top: show that truncation indicator ('+') consistently
With a little luck, this should be the final tweak for
our support of extra wide characters. Currently, those
characters don't always display the '+' indicator when
they've been truncated. Now, it should always be seen.

[ plus it's done a tad more efficiently via snprintf ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-03-03 17:52:43 +11:00
Jim Warner
0d352aa3d9 top: update copyright dates in source and man document
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
dddb8e1751 top: try to avoid premature truncation indicator ('+')
Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
4e77b307dc top: avoid potential truncation with 'Inspect' feature
As it turns out, that Ukrainian 'demo' text supporting
the '=' command was 152 bytes long, up from an English
version of 80 bytes. Unfortunately, the buffer used to
format all such strings was insufficient at 128 bytes.

Depending on the width of one's terminal, some strange
result could be experienced when a multi-byte sequence
was truncated. So, this just makes that buffer bigger.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
f3f90ab93c top: allow translated field headers to determine width
After wrestling with extra wide characters, supporting
languages like zh_CN, sometimes default/minimum column
widths might force a truncation of translated headers.

So, this commit explores one way that such truncations
could be avoided. It is designed so as to have minimal
impact on existing code, ultimately affecting just one
function. But it's off by default via its own #define.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
3b53aba319 top: an efficiency tweak to extra wide character logic
When I recently added extra wide character support for
locales like zh_CN, I didn't worry about some overhead
associated with the new calls to 'mbtowc' & 'wcwidth'.
That's because such overhead was usually incurred with
user interactions, not a normal iterative top display.

There was, however, one area where this overhead would
impact the normal iterative top mode - that's with the
Summary display. So I peeked at the glibc source code.

As it turns out, the costs of executing those 'mbtowc'
and 'wcwidth' functions were not at all insignificant.
So, this patch will avoid them in the vast majority of
instances, while still enabling extra wide characters.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
2167dcbccb top: standardize width of the %CPU & %MEM columns at 5
There is (should be) no justification for changing the
width of the percentage columns (%CPU, %MEM) depending
on the BOOST_PERCNT #define. So this patch will ensure
that both columns are fixed at their former maximum 5.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-19 20:37:24 +11:00
Jim Warner
c888534a4e top: account for the idle state ('I') threads in total
With the documentation update in the commit referenced
below, we should also account for such threads as they
will already be represented in the task/thread totals.

[ and do it in a way that might avoid future changes ]

Reference(s):
commit a238a687ce

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-12 20:53:57 +11:00
Jim Warner
b4bce3b8fa top: adapt utf8 logic to support extra wide characters
Back when top was refactored to support UTF-8 encoding
it was acknowledged that languages like zh_CN were not
supported. That was because a single 'character' might
require more than a single 'column' when it's printed.

Well I've now figured out how to accommodate languages
like that. My adaptation is represented in this patch.

[ and just in case someone wishes to avoid the extra ]
[ runtime costs, a #define OFF_XTRAWIDE is included. ]

Along the way, I've cleaned up some miscellaneous code
supporting the 'Inspect' feature so that the rightmost
screen column was always used rather than being blank.

[ interestingly, my xterm & urxvt terminal emulators ]
[ are able to split extra wide characters then print ]
[ 1/2 of such graphics in the last column. the gnome ]
[ terminal emulator does not duplicate such behavior ]
[ but prints 1 extra character in same width window. ]

Reference(s):
. Sep, 2017 - original utf8 support
commit 7ef38420a4

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-12 20:53:56 +11:00
Jim Warner
dfa118b4d5 top: tweak that recent enhancement to startup defaults
When the new approach for startup defaults was adopted
in the reference below, a file might be left open that
technically should be closed. This situation arises in
the unlikely event the #define RCFILE_NOERR is active.

Without that #define, the program will exit early thus
rendering the open file issue moot. However, even with
that #define there was no real harm with an open file.
It simply meant a 2nd FILE struct would have been used
when, or if, the rcfile was written via a 'W' command.

Anyway, this patch ensures such a file will be closed.

Reference(s):
. Dec, 2017 - /etc/topdefaultrc introduced
commit 3e6a208ae5

Signed-off-by: Jim Warner <james.warner@comcast.net>
2018-02-12 20:53:56 +11:00
Craig Small
a238a687ce docs: Document I idle state in ps and top
Linux 4.2 provided a new process state of I which is used for an idle
kernel thread. This new state means that kernel threads do not
contribute to the loadavg as they are no longer state D or S but I.

While both ps and top displayed this state, it wasn't documented in
either manual page until now.

References:
 https://bugs.debian.org/886967
 https://www.quora.com/What-does-mean-Linux-process-state-I-in-the-top-output
 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=80ed87c8a9ca0cad7ca66cf3bbdfb17559a66dcf
2018-01-13 11:18:09 +11:00
Jim Warner
3e6a208ae5 top: allow more flexible approach for startup defaults
Those references below offer more detail regarding the
default startup changes beginning with version 3.3.10.

It is important to remember that all such changes were
supposed to impact only new users or users who had not
saved the personal config file (via that 'W' command).
However, I introduced a bug wherein the rcfile was not
fully honored. This gave the changes a bad reputation.

That bug was corrected in release 3.3.11 but the issue
of default startup options keeps resurfacing. And it's
clear there's no consensus on what should be included.

Our --disable-modern-top configure option is of little
help since it remains an all-or-nothing approach. What
we need is an answer offering unlimited customization.
So, this commit will provide distribution packagers or
system administrators with a much more flexible way to
set their own preferred startup default configuration.

A new rcfile is being introduced: '/etc/topdefaultrc',
whose format/content is the same as a personal rcfile.
Thus once a 'proper' enterprise configuration has been
established and saved via 'W', it can be copied to the
/etc/ directory. Thereafter, startup in the absence of
a saved rcfile will use that configuration as default.

Now if a distribution packager or system administrator
wishes to expose their users to some of top's advanced
capabilities they can do so gradually. Perhaps setting
up graph mode for summary area task and memory display
while retaining the %CPU sort could be tried. Or maybe
showing colors, but better customized for a particular
terminal emulator. Such possibilities are now endless.

[ in exploiting this new capability, i hope that the ]
[ other windows (alt display mode) aren't overlooked ]

Reference(s):
. Sep, 2014 - Not fully honoring rcfile bug discussed
https://www.freelists.org/post/procps/top-saved-rcfile-bug
. Oct, 2014 - Attempt to defend new startup defaults
https://bugzilla.redhat.com/show_bug.cgi?id=1153049
. Jul, 2015 - Forest vs. %CPU views discussion
https://gitlab.com/procps-ng/procps/issues/6
. Oct, 2017 - Question the use of --disable-modern-top
https://bugzilla.redhat.com/show_bug.cgi?id=1499410
. Oct, 2017 - Forest vs. %CPU views discussion again
https://www.freelists.org/post/procps/Forest-mode-by-default-in-top-seems-a-bit-strange
. Dec, 2017 - Rehash of 3.3.10 startup defaults change
https://gitlab.com/procps-ng/procps/issues/78

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-12-23 17:41:37 +11:00
Jim Warner
cdf8eac99e top: miscellaneous changes to whitespace/comments only
[ ok, there's also 1 newly added #undef included too ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-12-23 17:41:37 +11:00
Jim Warner
18e5aecd2b top: exploit msg handler override to avoid corrupt tty
Way back in November of 2011, the library was equipped
with an overridable error message handler function. It
was done expressly for a program like top which alters
the tty. But that support was withdrawn shortly after.

This was all done in the lead up to v3.3.2. That's the
release where NLS support was added and it represented
a hectic time. In hindsight, the changes went too far.

So this commit, in a minimal fashion, restores ability
to address a potential fatal library error. After all,
any properly behaving library would never unilaterally
subject a caller to a stderr message and then an exit.

[ when exposing 1 variable in libprocps.sym, 2 other ]
[ existing symbols were repositioned alphabetically. ]

Reference(s):
. generalized library memory provisions
commit 7126cc4491
. top exploit library memory provisions
commit 88087ec5a5
. library xalloc type functions made private
commit 2865ded64e
. restored prior top memory logic
commit 05f5deb97c

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-12-20 21:07:44 +11:00
Jim Warner
0a91e21d3a top: add and/or expand a couple more translation hints
After noticing that the 'uk' translation expanded what
was supposed to be a 3 line header into 5 lines, seems
appropriate to offer more guidance on max lines count.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-12-20 21:07:35 +11:00
Jim Warner
4e2a1ba9b1 top: stop neglecting potential utf8 field descriptions
And I thought those strange characters I saw with only
certain translations in Fields Management descriptions
were resulting from my terminal emulator deficiencies.

Turns out that ol' top wasn't addressing possibilities
of such descriptions ending with multi-byte sequences.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-14 21:44:56 +11:00
Jim Warner
1ba0e999c6 top: eliminate that potential vulnerability for TOCTOU
Initially, I was going to ignore that coverity warning
CID #177876. But, since top may be running SETUID it's
best if it can be avoided instead. The fix was simple.

We'll trade the access() call for a real fopen() call.
This time-of-check-time-of-use warning should go away.
------------------------------------------------------

When XDG support was originally introduced in top, the
author made a poor choice in access(). A real question
that needed asking was 'does the file exist'. However,
the question that was asked was 'can this real user ID
or this real group ID access the file'. Then, when the
fopen() is finally issued, top would use the effective
user ID or the effective group ID to access that file.

That's what opened the potential TOCTOU vulnerability,
which was important only if top was running SUID/SGID.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-14 21:44:56 +11:00
Jim Warner
0f5d503103 top: make 'utf8_justify' independent of non-utf8 logic
By eliminating the call to 'fmtmk', the 'utf8_justify'
function could more easily be used in libproc someday.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-14 21:44:56 +11:00
Jim Warner
ac76afee36 top: utf8 utils should observe indentation conventions
Gosh, all this time we used indents of 4 spaces, not 3
spaces which were always the top standard indentation.

[ and we made our 'utf8_embody' a little more robust ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-14 21:44:56 +11:00
Jim Warner
fba7a919fb top: ensure bug report suggestion agrees with man page
The top man page was changed back on 10/20/15, in that
commit shown below. There, freelists.org was suggested
as the bug reports recipient. But, the program was not
changed from the original Debian bug reports approach.

Reference(s):
commit b1f7b2a509

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-14 21:44:56 +11:00
Jim Warner
f762cd5660 top: make the 'utf8_proper_col' routine more efficient
This patch better exploits short-circuit evaluation in
two 'if' tests. In every case, the 1st of 2 conditions
in each 'if' test must take place but it always proves
true with each iteration for 1 of the 'if' statements.
Thus, the 2nd condition will have to be evaluated too.

By reordering 2 tests in each 'if', we can ensure that
the 2nd condition will then be tested much less often.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-07 09:01:55 +11:00
Jim Warner
abde5d7d5e top: make that 'make_str_utf8' function more efficient
Upon reflection, there was absolutely no justification
for that call to strlen() which was then followed by a
call to snprintf(). The latter provides this needed #.

[ also make that 'delta' value a little more visible ]
[ instead of hiding it at the end of a its code line ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-07 09:01:55 +11:00
Jim Warner
6597ac227d top: ensured one translation hint agrees with template
Now that top can properly handle translated multi-byte
strings I've been reviewing translated efforts so far,
and weighing output against related translation hints.

In one case, a translation hint has not kept pace with
the current program state. In addition, that same hint
could be expanded to suggest translation alternatives.

[ frankly I never expected the translators to tackle ]
[ some of those 'special' strings. the task appeared ]
[ just too daunting. but they have done a great job! ]

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-07 09:01:55 +11:00
Jim Warner
18958c507e top: improve the translator hint regarding field width
Reference(s):
https://www.freelists.org/post/procps/Does-COMMAND-really-have-to-be-at-most-7-characters,2

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-07 09:01:55 +11:00
Jim Warner
f6ee2c0085 top: correct an insidious occasional truncation buglet
With the help of our Swedish translator, hopefully the
final buglet has now been vanquished in the multi-byte
translation support. This one was a real nasty bugger.

Although it didn't occur with every terminal emulator,
occasionally random text lines were being chopped off.

As it turns out, those terminals were blameless. There
were two separate places in top's show_special routine
where potential multi-byte sequences were inadequately
addressed. Solution: exploit existing utf-8 functions.

[ it also became apparent that the translation hints ]
[ in the top_nls module were deficient. so a special ]
[ caution was added regarding the final line of txt. ]

Reference(s):
https://gitlab.com/procps-ng/procps/issues/68

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-02 22:20:58 +11:00
Jim Warner
d5a9965cde top: plug a small potential hole in multi-byte support
Unlike the insp_mkrow_raw function the insp_mkrow_utf8
routine is not equipped to print non-ctl, non-printing
characters like '<7f>'. However, technically that very
value currently slips through the cracks. So with this
patch top will now print a space in the unlikely event
a character with the value of 127 is ever encountered.

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-02 22:20:58 +11:00
Jim Warner
cd88d4538b top: extend utf-8 multi-byte support to users & groups
Since all the necessary utf-8 plumbing is now in place
this commit will extend multi-byte support to user and
group names. Now top will be on a par with the ps guy.

[ plus, it's also my way of showing appreciation for ]
[ all those investments silently made by translators ]

Reference(s):
https://gitlab.com/procps-ng/procps/issues/68

Signed-off-by: Jim Warner <james.warner@comcast.net>
2017-10-01 22:30:16 +11:00