This is one of the worst issues that we found: if the strlen() of one of
the cmdline arguments is greater than INT_MAX (it is possible), then the
"int bytes" could wrap around completely, back to a very large positive
int, and the next strncat() would be called with a huge number of
destination bytes (a stack-based buffer overflow).
Fortunately, every distribution that we checked compiles its procps
utilities with FORTIFY, and the fortified strncat() detects and aborts
the buffer overflow before it occurs.
This patch also fixes a secondary issue: the old "--bytes;" meant that
cmdline[sizeof (cmdline) - 2] was never written to if the while loop was
never entered; in the example below, "ff" is the uninitialized byte:
((exec -ca `python3 -c 'print("A" * 131000)'` /usr/bin/cat < /dev/zero) | sleep 60) &
pgrep -a -P "$!" 2>/dev/null | hexdump -C
00000000 31 32 34 36 30 20 41 41 41 41 41 41 41 41 41 41 |12460 AAAAAAAAAA|
00000010 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 |AAAAAAAAAAAAAAAA|
*
00001000 41 41 41 41 ff 0a 31 32 34 36 32 20 73 6c 65 65 |AAAA..12462 slee|
00001010 70 20 36 30 0a |p 60.|
Otherwise (for example), if the (undocumented) opt_echo is set, but not
opt_long, and not opt_longlong, and not opt_pattern, there is a call to
xstrdup(cmdoutput) but cmdoutput was never initialized:
sleep 60 & echo "$!" > pidfile
env -i LD_DEBUG=`perl -e 'print "A" x 131000'` pkill -e -c -F pidfile | xxd
...
000001c0: 4141 4141 4141 4141 4141 4141 4141 4141 AAAAAAAAAAAAAAAA
000001d0: 4141 4141 4141 4141 fcd4 e6bd e47f 206b AAAAAAAA...... k
000001e0: 696c 6c65 6420 2870 6964 2031 3230 3931 illed (pid 12091
000001f0: 290a 310a ).1.
[1]+ Terminated sleep 60
(the LD_DEBUG is just a trick to fill the initial stack with non-null
bytes, to show that there is uninitialized data from the stack in the
output; here, an address "fcd4 e6bd e47f")
Not exploitable (not under an attacker's control), but still a potential
non-security problem. Copied, fixed, and used the grow_size() macro from
pidof.c.
memset()ing task and subtask inside their loops prevents free_acquired()
(in readproc() and readtask()) from free()ing their contents (especially
cmdline and environ).
Our solution is not perfect, because we still memleak the very last
cmdline/environ, but select_procs() is called only once, so this is not
as bad as it sounds.
It would be better to leave subtask in its block and call
free_acquired() after the loop, but this function is static (not
exported).
The only other solution is to use freeproc(), but this means replacing
the stack task/subtask with xcalloc()s, thus changing a lot of code in
pgrep.c (to pointer accesses).
Hence this imperfect solution for now.
sig.c had this odd logic where on non-Hurd systems it would undefine
SIGLOST. Fine for Hurd or amd64 Linux systems. Bad for a sparc which
has SIGLOST defined *and* is not Hurd.
Just check its defined, its much simpler.
If pgrep is run with a non-program name match and there are
no matches, it segfaults.
The testsuite thinks zero bytes sent, and zero bytes sent
because the program crashed is the same :/
References:
commit 1aacf4af7f199d77fc9386e249eee654f59880db
https://bugs.debian.org/894917
Signed-off-by: Craig Small <csmall@enc.com.au>
Update NEWS with the version
Add library API change into NEWS
Update c:r:a for library to 7:0:1
This means the current and age are incremented, so old programs can
use new library but not vice-versa as they won't have the numa*
functions.
pidof will miss scripts that are run a certain way due to how
they appear in procfs. This is just a note to say it might miss
them.
References:
procps-ng/procps#17
Hurd doesn't have HOST_NAME_MAX, neither does Solaris.
An early fix just checked for this value and used 64 instead.
This change uses sysconf which is the correct method, possibly until
this compiles on some mis-behaving OS which doesn't have this value.
References:
commit e564ddcb01c3c11537432faa9c7a7a6badb05930
procps-ng/procps#54
By default pgrep/pkill should not kill processes in a namespace it is not
part of. If this is allowed, it allows callers to break namespaces they did
not expect to affect, requiring rewrite of all callers to fix.
So by default, we should work in the current namespace. If --ns 0 is
specified, they we look at all namespaces, and if any other pid is specified
we continue to look in only that namespace.
Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
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>
The previous commit had one minor bug in it because the fields need
to be alphabetical and times comes after timeout.
Added NEWS item for this feature
Added another testsuite check for new flags in case they
disappear or go strange one day.
References:
commit 8a94ed61119f8dcf7bcb98b84534e408d4eb7769
I frequency use pidof command with strace system call tracer.
strace can trace MULTIPLE processes specified with "-p $PID"
arguments like:
strace -p 1 -p 1030 -p 3043
Sometimes I want to do as following
strace -p $(pidof httpd)
However, above command line doesn't work because -p option
is needed for specifying a pid. pidof uses a whitespace as
a separator. For passing the output to strace, the separator
should be replaced with ' -p '.
This maybe not a special to my use case.
This commit introduces -S option that allows a user to specify a
separator the one wants.
$ ./pidof bash
./pidof bash
24624 18790 12786 11898 11546 10766 7654 5095
$ ./pidof -S ',' bash
./pidof -S ',' bash
24624,18790,12786,11898,11546,10766,7654,5095
$ ./pidof -S '-p ' bash
./pidof -S '-p ' bash
24624-p 18790-p 12786-p 11898-p 11546-p 10766-p 7654-p 5095
$ ./pidof -S ' -p ' bash
./pidof -S ' -p ' bash
24624 -p 18790 -p 12786 -p 11898 -p 11546 -p 10766 -p 7654 -p 5095
$ strace -p $(./pidof -S ' -p ' bash)
strace -p $(./pidof -S ' -p ' bash)
strace: Process 24624 attached
strace: Process 18790 attached
strace: Process 12786 attached
...
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
The procio functions that were in the library have been
moved into sysctl. sysctl is not linked to libprocps in
newlib and none of the other procps binaries would need
to read/write large data to the procfs.
References:
be6b048a41b0a47ebed602d9e0993fe18c9de237
thereby use one allocated buffer for I/O which now might
be increased by the stdio function getline(3) on the
file if required.
Signed-off-by: Werner Fink <werner@suse.de>
to be able to read and write large buffers below /proc.
The buffers and file offsets are handled dynamically
on the required buffer size at read, that is lseek(2)
is used to determine this size. Large buffers at
write are split at a delimeter into pieces and also
lseek(2) is used to write each of them.
Signed-off-by: Werner Fink <werner@suse.de>
by using getline(3) to use a dynamically increased buffer
if required by the input found in sysctl configuration files.
Signed-off-by: Werner Fink <werner@suse.de>
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>
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>
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>
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>
