From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26472 invoked by alias); 2 Aug 2009 21:11:32 -0000 Received: (qmail 26457 invoked by uid 22791); 2 Aug 2009 21:11:31 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_44,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 02 Aug 2009 21:11:25 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n72LAjeP009963 for ; Sun, 2 Aug 2009 17:10:55 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n72LAdus018705 for ; Sun, 2 Aug 2009 17:10:44 -0400 Received: from host0.dyn.jankratochvil.net (sebastian-int.corp.redhat.com [172.16.52.221]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n72LAblk030812 for ; Sun, 2 Aug 2009 17:10:38 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n72LAb7Q028430; Sun, 2 Aug 2009 23:10:37 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id n72LAbVr028429; Sun, 2 Aug 2009 23:10:37 +0200 Date: Sun, 02 Aug 2009 21:11:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Cc: Richard Guenther Subject: [patch] Fix find_separate_debug_file buffer overrun [Re: gdb crash during read of separate debuginfo] Message-ID: <20090802211036.GA28064@host0.dyn.jankratochvil.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.19 (2009-01-05) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-08/txt/msg00013.txt.bz2 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 Jan Kratochvil 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 * 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 . + +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"