Patchwork avoid memory overrun in a test leading to potential double-free

login
register
mail settings
Submitter Jim Meyering
Date March 8, 2011, 12:59 p.m.
Message ID <87aah5rco9.fsf@rho.meyering.net>
Download mbox | patch
Permalink /patch/85973/
State New
Headers show

Comments

Jim Meyering - March 8, 2011, 12:59 p.m.
I ran "make check" and was dismayed to see that glibc detected a
double-free.  At first I thought it must be my fault, since I'd been
removing useless tests before free, but no...

Running "valgrind ./test-expandargv" confirmed it:

  ==29710== Conditional jump or move depends on uninitialised value(s)
  ==29710==    at 0x400E14: run_replaces (test-expandargv.c:121)
  ==29710==    by 0x400F63: writeout_test (test-expandargv.c:151)
  ==29710==    by 0x401037: run_tests (test-expandargv.c:188)
  ==29710==    by 0x40124C: main (test-expandargv.c:264)


From f60778ef0f07983b0ba72ed97fe52b687de28abb Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Tue, 8 Mar 2011 13:54:13 +0100
Subject: [PATCH] avoid memory overrun in a test leading to potential double-free

* testsuite/test-expandargv.c (writeout_test): Fix off-by-one error:
i.e., do copy the trailing NUL byte.
---
 libiberty/ChangeLog                   |    6 ++++++
 libiberty/testsuite/test-expandargv.c |    2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)
DJ Delorie - March 8, 2011, 7:23 p.m.
> avoid memory overrun in a test leading to potential double-free
> * testsuite/test-expandargv.c (writeout_test): Fix off-by-one error:
> i.e., do copy the trailing NUL byte.

Ok.  Thanks!
Mike Stump - April 10, 2011, 6:16 p.m.
On Mar 8, 2011, at 11:23 AM, DJ Delorie wrote:
>> avoid memory overrun in a test leading to potential double-free
>> * testsuite/test-expandargv.c (writeout_test): Fix off-by-one error:
>> i.e., do copy the trailing NUL byte.
> 
> Ok.  Thanks!

Applied, thanks.  In general, you have to ask someone to apply things for you, if you can't actually do the checkin, after a patch is approved.

Patch

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index dc92638..802cf96 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,6 +1,12 @@ 
+2011-03-08  Jim Meyering  <meyering@redhat.com>
+
+	avoid memory overrun in a test leading to potential double-free
+	* testsuite/test-expandargv.c (writeout_test): Fix off-by-one error:
+	i.e., do copy the trailing NUL byte.
+
 2011-02-28  Kai Tietz  <kai.tietz@onevision.com>

 	* filename_cmp.c (filename_ncmp): New function.
 	* functions.texi: Regenerated.

 2011-02-03  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>
diff --git a/libiberty/testsuite/test-expandargv.c b/libiberty/testsuite/test-expandargv.c
index c16a032..57b96b3 100644
--- a/libiberty/testsuite/test-expandargv.c
+++ b/libiberty/testsuite/test-expandargv.c
@@ -201,13 +201,13 @@  writeout_test (int test, const char * test_data)
   /* Generate RW copy of data for replaces */
   len = strlen (test_data);
   parse = malloc (sizeof (char) * (len + 1));
   if (parse == NULL)
     fatal_error (__LINE__, "Failed to malloc parse.", errno);
       
-  memcpy (parse, test_data, sizeof (char) * len);
+  memcpy (parse, test_data, sizeof (char) * (len + 1));
   /* Run all possible replaces */
   run_replaces (parse);

   fwrite (parse, len, sizeof (char), fd);
   free (parse);
   fclose (fd);