* [patch] Fix find_separate_debug_file buffer overrun [Re: gdb crash during read of separate debuginfo]
[not found] <alpine.LNX.2.00.0908021137300.16347@zhemvz.fhfr.qr>
@ 2009-08-02 21:11 ` Jan Kratochvil
2009-08-03 16:46 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kratochvil @ 2009-08-02 21:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Richard Guenther
Hi Richard,
On Sun, 02 Aug 2009 11:40:12 +0200, Richard Guenther wrote:
>
> We experienced crashes when running gdb inside out installation
> system which has /usr symlinked to some location beyond /mnt.
> The issue is that the code doesn't deal with the case that
> the result of lrealpath is longer than its argument.
thanks, posting updated patch for FSF GDB as it is not a Fedora regression.
This attached patch has not been reviewed by Richard Guenther.
The testcase would no longer reproduce the bug after some patch like this one
would get accepted as the IMO only exploitable code path gets removed:
Re: [patch] Replace reread_symbols by load+free calls
http://sourceware.org/ml/gdb-patches/2009-06/msg00696.html
At the same time this reread_symbols patch fixes some bug not further
investigated currently exposed by commented-out runto_main in this testcase.
I do not push much to get the testcase accepted.
No regressions on {x86_64,x86_64-m32,i686}-fedora11-linux-gnu.
Thanks,
Jan
gdb/
2009-08-02 Richard Guenther <rguenther@suse.de>
Jan Kratochvil <jan.kratochvil@redhat.com>
Fix memory corruption on reread of file through a symbolic link.
* symfile.c (find_separate_debug_file): Initialize CANON_NAME earlier.
Allocate DEBUGFILE with length based on CANON_NAME. Free CANON_NAME on
all the return paths.
gdb/testsuite/
2009-08-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/debuginfo-lrealpath-overflow.exp: New.
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1388,8 +1388,14 @@ find_separate_debug_file (struct objfile *objfile)
gdb_assert (i >= 0 && IS_DIR_SEPARATOR (dir[i]));
dir[i+1] = '\0';
+ /* Set I to max (strlen (canon_name), strlen (dir)). */
+ canon_name = lrealpath (dir);
+ i = strlen (dir);
+ if (canon_name && strlen (canon_name) > i)
+ i = strlen (canon_name);
+
debugfile = alloca (strlen (debug_file_directory) + 1
- + strlen (dir)
+ + i
+ strlen (DEBUG_SUBDIRECTORY)
+ strlen ("/")
+ strlen (basename)
@@ -1403,6 +1409,7 @@ find_separate_debug_file (struct objfile *objfile)
{
xfree (basename);
xfree (dir);
+ xfree (canon_name);
return xstrdup (debugfile);
}
@@ -1416,6 +1423,7 @@ find_separate_debug_file (struct objfile *objfile)
{
xfree (basename);
xfree (dir);
+ xfree (canon_name);
return xstrdup (debugfile);
}
@@ -1429,12 +1437,12 @@ find_separate_debug_file (struct objfile *objfile)
{
xfree (basename);
xfree (dir);
+ xfree (canon_name);
return xstrdup (debugfile);
}
/* If the file is in the sysroot, try using its base path in the
global debugfile directory. */
- canon_name = lrealpath (dir);
if (canon_name
&& strncmp (canon_name, gdb_sysroot, strlen (gdb_sysroot)) == 0
&& IS_DIR_SEPARATOR (canon_name[strlen (gdb_sysroot)]))
@@ -1449,6 +1457,7 @@ find_separate_debug_file (struct objfile *objfile)
xfree (canon_name);
xfree (basename);
xfree (dir);
+ xfree (canon_name);
return xstrdup (debugfile);
}
}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/debuginfo-lrealpath-overflow.exp
@@ -0,0 +1,67 @@
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+set test debuginfo-lrealpath-overflow
+set binfile ${objdir}/${subdir}/${test}
+set binfileloadbase ${test}-load
+set binfileload ${objdir}/${subdir}/${binfileloadbase}
+
+# Length 250 should be under NAME_MAX and still big enough for a stack overflow.
+set pattern [string repeat 0123456789 25]
+set longdirbase [string range ${test}-${pattern} 0 249]
+set longdir ${objdir}/${subdir}/${longdirbase}
+
+if {[build_executable ${test}.exp ${test} start.c debug] == -1} {
+ return -1
+}
+
+# find_separate_debug_file is during the initial load called by
+# symbol_file_add_with_addrs_or_offsets on objfile->name which is already the
+# resolved long name. Use the reread_separate_symbols caller with
+# objfile->name created at time when still the long directory name and symlink
+# still did not exist.
+
+remote_exec build "rm -f ${binfileload}"
+remote_exec build "mkdir -p ${binfileload}"
+remote_exec build "mv -f ${binfile} ${binfileload}/${test}"
+
+clean_restart ${binfileloadbase}/${test}
+
+remote_exec build "rm -rf ${longdir}"
+remote_exec build "mv -f ${binfileload} ${longdir}"
+remote_exec build "ln -s ${longdirbase} ${binfileload}"
+
+# Note: the procedure gdb_gnu_strip_debug will produce an executable called
+# ${binfile}, which is just like the executable ($binfile) but without
+# the debuginfo. Instead $binfile has a .gnudebuglink section which contains
+# the name of a debuginfo only file. This file will be stored in the
+# gdb.base/.debug subdirectory.
+
+if [gdb_gnu_strip_debug ${binfileload}/${test}] {
+ # check that you have a recent version of strip and objcopy installed
+ unsupported "cannot produce separate debug info files"
+ return -1
+}
+
+# Delete the separate debug info file to call lrealpath at all.
+remote_exec build "rm -f ${binfileload}/.debug/${test}.debug"
+
+sleep 1
+remote_exec build "touch ${binfileload}/${test}"
+
+# runto_main could expose a reread_symbols bug giving false FAIL here.
+
+gdb_run_cmd
+gdb_test "" "" "catch the prompt"
^ permalink raw reply [flat|nested] 3+ messages in thread