coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coreutils] [PATCH] csplit: avoid buffer overrun when writing more t


From: Pádraig Brady
Subject: Re: [coreutils] [PATCH] csplit: avoid buffer overrun when writing more than 999 files
Date: Wed, 24 Nov 2010 01:21:01 +0000
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

On 10/11/10 13:49, Pádraig Brady wrote:
> On 10/11/10 13:25, Jim Meyering wrote:
>> Here's a patch.
>>
>> The included test is a little unusual.
>> Unlike most such tests, this one does not fail without the fix.
>> However, running it against a valgrind-wrapped does expose the bug.
>>
>> Any suggestions for a better way (even O/S- or kernel-specific)
>> to test this would be most welcome.  While I'm inclined not
>> to run valgrind directly (I run it periodically on everything,
>> via wrappers), this is one possibility:
>>
>>     seq 1000 | valgrind --error-exitcode=1 -- csplit - '/./' '{*}' || fail=1
>>
>> But that would involve first ensuring that it's installed and usable.
> 
> On a related note, valgrind misses overwritten buffers on the stack.
> I wonder would there be a way to coerce the compiler into
> mallocing such buffers and auto freeing them upon leaving scope?

I just noticed `valgrind --tool=exp-ptrcheck` which is supposed
to check stack buffers. I found it produced both false positives
and negatives and results vary across runs, so it's fairly
unusable at this stage.

So I tried a different method which was much more effective.
Simply change the stack bufs to the heap (ignoring leaks) like:
  sed -i "s/\(^ \+char\) \([a-z_]\+\)\[\([^]]\+\)\];/\1 *\2 = malloc(\3);/" *.c
  (Need to manually fix up the few instances within structs
   and the couple of sizeof gotchas).
I then ran that through the default memcheck tool,
which did detect manually introduced overflows,
and did not detect any overflows in our existing code :)

BTW, README-valgrind has gotten a little out of date,
so I'll update with the following...

diff --git a/README-valgrind b/README-valgrind
index 151ec2d..2bea955 100644
--- a/README-valgrind
+++ b/README-valgrind
@@ -16,26 +16,34 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

+
+
 # Convert Makefile.am files:
-#  find tests -name Makefile.am | xargs grep -wl PATH|xargs perl -pi -e \
-#    's,src(\$\(PATH_SEPARATOR\)\$\$PATH),src/vg$1,'
+#  find tests -name check.mk | xargs grep -wl PATH |
+#    xargs perl -pi -e 's,src(\$\(PATH_SEPARATOR\)),src/vg$1,'
 # To restore:
-# find tests -name Makefile.am|xargs grep -wl PATH|xargs perl -pi -e 
's,src/vg,src,'
+#  find tests -name check.mk | xargs grep -wl PATH |
+#    xargs perl -pi -e 's,src/vg,src,'
 #
 # Create this symlink for suppressions (this is no longer necessary,
 # with Linux kernel 2.6.9 and valgrind-2.2.0):
 # ln -s $PWD/.vg-suppressions /tmp/cu-vg

+
 # Create src/vg:

-coreutils=$(echo 'spy:;@echo $(all_programs)' | (cd src; make -f Makefile -f - 
spy | tr -s '\n ' '  '))
+coreutils=$(echo 'spy:;@echo $(all_programs) $(noinst_PROGRAMS)' |
+            (cd src; make -f Makefile -f - spy | tr -s '\n ' '  '))
 mkdir -p src/vg
 pwd=`pwd`
 srcdir=$pwd/src
 _path='export PATH='$srcdir':${PATH#*:}'
 pre='#!/bin/sh\n'"$_path"'\n'
-n=15
-vg='exec /usr/bin/valgrind --suppressions=/tmp/cu-vg --log-fd=3 
--leak-check=yes --track-fds=yes --leak-check=full --num-callers='$n
+n=15 # stack trace depth
+log_fd=3 # One can redirect this to file like 3>vg.log
+test -e /tmp/cu-vg && suppressions='--supressions=/tmp/cu-vg'
+vg="exec /usr/bin/valgrind $suppressions --log-fd=$log_fd \
+--leak-check=yes --track-fds=yes --leak-check=full --num-callers=$n"
 cat <<EOF > src/vg/gen
 for i in $coreutils; do
   printf "$pre$vg -- \$i"' "\$@"\n' > \$i



reply via email to

[Prev in Thread] Current Thread [Next in Thread]