bugmake - Bugs: bug #44660, possible buffer overflow?

 
 

bug #44660: possible buffer overflow?

Submitter:  James Michael DuPont <mdupont>
Submitted:  Sun 29 Mar 2015 09:48:55 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Duplicate Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  4.0 Operating System:  POSIX-Based
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 12 Jul 2015 05:45:33 PM UTC, comment #3: 

Thanks, this is the same cause as bug #45275

Paul D. Smith <psmith>
Group administrator
Sun 29 Mar 2015 06:18:45 PM UTC, comment #2: 

Thanks for the repro case and investigation.  I'll look into this.

Paul D. Smith <psmith>
Group administrator
Sun 29 Mar 2015 06:11:10 PM UTC, comment #1: 

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

martind@swiftboat:~/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:

martind@swiftboat:~/tmp/make-44660$ cat Makefile
o : $(subst A,AA,$(subst A,AAAAAAAA,$(subst A,AAAAAAAA,$(subst A,AAAAAAAA,AAAAAAAA))));
martind@swiftboat:~/tmp/make-44660$ ruby -we 'puts(8*8*8*8*2)'
8192
martind@swiftboat:~/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:

martind@swiftboat:~/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 ();
martind@swiftboat:~/download/make-git$

I think it was a regression under:

Differences between revisions 9903cda2a734c2f86eefcff656aad032fbb79078 and 1454a04f81708850353dbdc0807a099c5aaab55b:

2011-02-21 07:30:11 +0000 -email is unavailable- (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


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

Martin Dorey <mdorey>
Sun 29 Mar 2015 09:48:55 AM UTC, original submission:  

this is a badly generated makefile from a program that I am developing. I hope this might help improve make.

GNU Make 4.0, Built for powerpc64le-redhat-linux-gnu, make-4.0-3.fc21.ppc64le

make -f Makefile.test GOPATH=~/testgo2/src
Reading makefile 'Makefile.test'...
Segmentation fault (core dumped)

I assume it is some buffer overflow :

source of makefile
https://raw.githubusercontent.com/h4ck3rm1k3/go/makefile-gen/src/Makefile.test


James Michael DuPont <mdupont>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #33477:  strcache.c.take2.patch added by mdorey (787B - application/octet-stream - the same patch as in my first post)
file #33476:  Makefile.test added by mdupont (16KiB - application/octet-stream - nasty makefile)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by mdorey (Posted a comment)
  • -email is unavailable- added by mdupont (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-07-12 psmith StatusNone Duplicate
        Open/ClosedOpen Closed
    2015-03-29 mdorey Attached File- Added strcache.c.take2.patch, #33477
    2015-03-29 mdupont Attached File- Added Makefile.test, #33476

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code