OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix crash on process name "(sd-pam)" (PR 16594).

Browse Source 
info os processes -fsanitize=address error
https://sourceware.org/bugzilla/show_bug.cgi?id=16594

info os processes
=================================================================
==5795== ERROR: AddressSanitizer: heap-use-after-free on address
0x600600214974 at pc 0x757a92 bp 0x7fff95dd9f00 sp 0x7fff95dd9ef0
READ of size 4 at 0x600600214974 thread T0
    #0 0x757a91 in get_cores_used_by_process (.../gdb/gdb+0x757a91)

At least Fedora 20 has process(es):
 6678 ?        Ss     0:00 /usr/lib/systemd/systemd --user
 6680 ?        S      0:00  \_ (sd-pam)

and GDB "info os processes" crashes on it as /proc/6680/stat contains:

6680 ((sd-pam)) S 6678 6678 6678 0 -1 1077961024 33 0 0 0 0 0 0 0 20 0 1 0 18568 73768960 120 18446744073709551615 1 1
0 0 0 0 0 4096 0 18446744073709551615 0 0 17 6 0 0 0 0 0 0 0 0 0 0 0 0 0

and GDB fails to find the proper end of the process name "((sd-pam))".
Therefore it reads core number off-by-one (it reads 17 instead of 6) and
overruns the array.

(1) Make the process name parsing more foolproof.

(2) Do not trust the parsed number from /proc/PID/stat and verify it against
    the array size.

I noticed that 'ps' gets this right, so I've peeked at its
sources, and it just looks for the first ')' starting at
the end.

dc072aced7:proc/readproc.c

Look for stat2proc.

Given ps does that, I believe the kernel won't ever be changed
in a way that would break it.  So it sounds like could do strrchr
from the end of stat just as well without worry, which is simpler.

gdb/
2014-02-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR gdb/16594
	* common/linux-osdata.c (linux_common_core_of_thread): Find the end of
	process name.
	(get_cores_used_by_process): New parameter num_cores, use it.
	(linux_xfer_osdata_processes): Pass num_cores to it.
	* linux-tdep.c (linux_info_proc, linux_fill_prpsinfo): Find the end of
	process name.

Message-ID: <20140217212826.GA15080@host2.jankratochvil.net>
This commit is contained in:
Jan Kratochvil 2014-02-21 18:39:40 +01:00
parent dcf893b581
commit 184cd07257
3 changed files with 27 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
gdb/ChangeLog
View File

@ -1,3 +1,13 @@
2014-02-21 Jan Kratochvil <jan.kratochvil@redhat.com>
PR gdb/16594
* common/linux-osdata.c (linux_common_core_of_thread): Find the end of
process name.
(get_cores_used_by_process): New parameter num_cores, use it.
(linux_xfer_osdata_processes): Pass num_cores to it.
* linux-tdep.c (linux_info_proc, linux_fill_prpsinfo): Find the end of
process name.
2014-02-21 Andreas Arnez <arnez@vnet.linux.ibm.com>
* target.c (memory_xfer_partial): Fix length arg in call to

16
gdb/common/linux-osdata.c
View File

@ -96,11 +96,8 @@ linux_common_core_of_thread (ptid_t ptid)
}
}
p = strchr (content, '(');
/* Skip ")". */
if (p != NULL)
p = strchr (p, ')');
/* ps command also relies on no trailing fields ever contain ')'. */
p = strrchr (content, ')');
if (p != NULL)
p++;
@ -258,11 +255,10 @@ get_process_owner (uid_t *owner, PID_T pid)
}
/* Find the CPU cores used by process PID and return them in CORES.
CORES points to an array of at least sysconf(_SC_NPROCESSOR_ONLN)
elements. */
CORES points to an array of NUM_CORES elements. */
static int
get_cores_used_by_process (PID_T pid, int *cores)
get_cores_used_by_process (PID_T pid, int *cores, const int num_cores)
{
char taskdir[sizeof ("/proc/") + MAX_PID_T_STRLEN + sizeof ("/task") - 1];
DIR *dir;
@ -286,7 +282,7 @@ get_cores_used_by_process (PID_T pid, int *cores)
core = linux_common_core_of_thread (ptid_build ((pid_t) pid,
(pid_t) tid, 0));
if (core >= 0)
if (core >= 0 && core < num_cores)
{
++cores[core];
++task_count;
@ -350,7 +346,7 @@ linux_xfer_osdata_processes (gdb_byte *readbuf,
/* Find CPU cores used by the process. */
cores = (int *) xcalloc (num_cores, sizeof (int));
task_count = get_cores_used_by_process (pid, cores);
task_count = get_cores_used_by_process (pid, cores, num_cores);
cores_str = (char *) xcalloc (task_count, sizeof ("4294967295") + 1);
for (i = 0; i < num_cores && task_count > 0; ++i)

18
gdb/linux-tdep.c
View File

@ -476,7 +476,9 @@ linux_info_proc (struct gdbarch *gdbarch, char *args,
p = skip_spaces_const (p);
if (*p == '(')
{
const char *ep = strchr (p, ')');
/* ps command also relies on no trailing fields
ever contain ')'. */
const char *ep = strrchr (p, ')');
if (ep != NULL)
{
printf_filtered ("Exec file: %.*s\n",
@ -1331,12 +1333,14 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
proc_stat = skip_spaces (proc_stat);
/* Getting rid of the executable name, since we already have it. We
know that this name will be in parentheses, so we can safely look
for the close-paren. */
while (*proc_stat != ')')
++proc_stat;
++proc_stat;
/* ps command also relies on no trailing fields ever contain ')'. */
proc_stat = strrchr (proc_stat, ')');
if (proc_stat == NULL)
{
do_cleanups (c);
return 1;
}
proc_stat++;
proc_stat = skip_spaces (proc_stat);