[bug #44660] possible buffer overflow?

[Martin Dorey]

Follow-up Comment #1, bug #44660 (project make):

Reproduced on amd64 with up-to-the-minute make from git.  valgrind reports
things going south starting here:

address@hidden:~/tmp/make-44660$ valgrind ~/download/make-git/make
==30211== Memcheck, a memory error detector
==30211== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==30211== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==30211== Command: /home/martind/download/make-git/make
==30211== 
==30211== Invalid write of size 1
==30211==    at 0x4C2B614: memmove (mc_replace_strmem.c:981)
==30211==    by 0x421FE5: add_hash (strcache.c:105)
==30211==    by 0x41BB8D: parse_file_seq (read.c:3342)
==30211==    by 0x40D222: split_prereqs (file.c:448)
==30211==    by 0x41AC47: record_files (read.c:1993)
==30211==    by 0x41C787: eval (read.c:1402)
==30211==    by 0x41DD80: eval_makefile (read.c:446)
==30211==    by 0x41E13B: read_all_makefiles (read.c:263)
==30211==    by 0x407914: main (main.c:1991)
==30211==  Address 0x580c8c0 is 0 bytes after a block of size 8,176 alloc'd
==30211==    at 0x4C28BED: malloc (vg_replace_malloc.c:263)
==30211==    by 0x417F98: xmalloc (misc.c:220)
==30211==    by 0x4220AC: add_hash (strcache.c:63)
==30211==    by 0x422218: strcache_add_len (strcache.c:207)
==30211==    by 0x41B708: construct_include_path (read.c:2893)
==30211==    by 0x4073ED: main (main.c:1796)

A simpler reproducer:

address@hidden:~/tmp/make-44660$ cat Makefile
o : $(subst A,AA,$(subst A,AAAAAAAA,$(subst A,AAAAAAAA,$(subst
A,AAAAAAAA,AAAAAAAA))));
address@hidden:~/tmp/make-44660$ ruby -we 'puts(8*8*8*8*2)'
8192
address@hidden:~/tmp/make-44660$ valgrind ~/download/make-git/make
==32079== Memcheck, a memory error detector
==32079== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==32079== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==32079== Command: /home/martind/download/make-git/make
==32079== 
==32079== Invalid write of size 8
==32079==    at 0x4C2B5A3: memmove (mc_replace_strmem.c:981)
==32079==    by 0x421FE5: add_hash (strcache.c:105)
==32079==    by 0x41BB8D: parse_file_seq (read.c:3342)
==32079==    by 0x40D222: split_prereqs (file.c:448)
==32079==    by 0x41AC47: record_files (read.c:1993)
==32079==    by 0x41C787: eval (read.c:1402)
==32079==    by 0x41DD80: eval_makefile (read.c:446)
==32079==    by 0x41E13B: read_all_makefiles (read.c:263)
==32079==    by 0x407914: main (main.c:1991)

Remove one of the first pair of As and the crash stops happening, so it's
triggered somewhere between 4 KiB and 8 KiB.

This seems to fix that example for me:

address@hidden:~/download/make-git$ git diff
diff --git a/strcache.c b/strcache.c
index 1ade5e7..7f71544 100644
--- a/strcache.c
+++ b/strcache.c
@@ -76,7 +76,7 @@ static const char *
 add_string (const char *str, unsigned int len)
 {
   char *res;
-  struct strcache *sp;
+  struct strcache *sp = NULL;
   struct strcache **spp = &strcache;
   /* We need space for the nul char.  */
   unsigned int sz = len + 1;
@@ -89,11 +89,12 @@ add_string (const char *str, unsigned int len)
   else
     /* Find the first cache with enough free space.  */
     for (; *spp != NULL; spp = &(*spp)->next)
-      if ((*spp)->bytesfree > sz)
+      if ((*spp)->bytesfree > sz) {
+        sp = *spp;
         break;
+      }
 
   /* If nothing is big enough, make a new cache.  */
-  sp = *spp;
   if (sp == NULL)
     {
       sp = new_cache ();
address@hidden:~/download/make-git$ 

I think it was a regression under:

Differences between revisions 9903cda2a734c2f86eefcff656aad032fbb79078 and
1454a04f81708850353dbdc0807a099c5aaab55b:

2011-02-21 07:30:11 +0000 address@hidden
(1454a04f81708850353dbdc0807a099c5aaab55b)

* Fixups to the make man page * Minor syntax cleanups in the manual * In
non-maintainer mode set NDEBUG to disable assert() * Performance improvements
in strcache:     Build Info                     1000    2000    4000     3.82 
-g                        2.61s   8.85s   33.52s 
   3.82 -O2                     1.90s   7.62s   27.82s     New -g (with 
asserts)        1.03s   2.31s   5.79s  
  New -O2 (no asserts)  0.65s   1.50s   3.52s

---------------------------------------------------


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?44660>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/