Re: [patch] m4 memory corruption bug

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to M. Levinson on 2/24/2007 5:30 PM:
> The GET_STRING macro in src/freeze.c from the current CVS sources has a
> trivial bug that can cause reload_frozen_state() to reuse freed memory: If
> CHECK_ALLOCATION modifies the temporary pointer p to point at newly-allocated
> memory, GET_STRING's original pointer Buf still points at the old block of
> memory that CHECK_ALLOCATION has freed.
> 
> Below is a patch that fixes the problem, but I don't think I can provide
> a test case that will demonstrate the bug in a portable way. On my system
> m4 frequently segfaults when invoked with the -R option, but of course on
> any other system the results for the same frozen state files will differ
> depending on exactly how and when the freed memory gets reused.

Thanks for the patch.  I take it this is for CVS head, and not branch-1_4.
 And the problem has only been there for 5 weeks, so it is not worth a
news entry.  I would like to list you in the THANKS file; do you have a
preferred name beyond M. Levinson?

> 
> 
> --- src/freeze.c      2007/02/05 17:48:11     1.1
> +++ src/freeze.c      2007/02/24 22:56:10
> @@ -414,8 +414,9 @@
>    do                                                         \
>      {                                                                \
>        size_t len = (StrLen);                                 \
> -      char *p = (Buf);                                               \
> -      CHECK_ALLOCATION (p, (BufSize), len);                  \
> +      char *p;                                                       \
> +      CHECK_ALLOCATION ((Buf), (BufSize), len);                      \
> +      p = (Buf);                                             \
>        while (len-- > 0)                                              \
>       {                                                       \
>         int ch = (version > 1 ? decode_char (File)            \
> 

I'm also checking this in as a test case.  While it doesn't have the same
reaction on every platform, it at least failed on cygwin prior to your
patch, which gives some assurance that we can avoid regressions in this area.

2007-02-25  Eric Blake  <address@hidden>

        * tests/freeze.at (loading format 2): Augment test to catch
        regression introduced on 2007-01-20.
        * THANKS: Update.
        Reported by M. Levinson.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF4hF484KuGfSFAYARAuOMAKC5FiWjMUmxw+jxYkEMrSnyyZFBDQCgn9wd
MbrN9B0vI+WDzT3t7Lmt5D8=
=K0vv
-----END PGP SIGNATURE-----

Index: tests/freeze.at
===================================================================
RCS file: /sources/m4/m4/tests/freeze.at,v
retrieving revision 1.9
diff -u -p -r1.9 freeze.at
--- tests/freeze.at     23 Jan 2007 14:28:23 -0000      1.9
+++ tests/freeze.at     25 Feb 2007 22:42:36 -0000
@@ -131,6 +131,12 @@ builtinbuiltingnu
 # Testing escape sequences
 T4,5
 blah\t\477\040\X5C
+# Long macro definition.  Catches a regression introduced on 2007-01-20
+# and patched 2007-02-25.
+T4,122
+long01234567890123456789012345678901234567890123456789
+01234567890123456789012345678901234567890123456789
+01234567890123456789
 ]])
 
 AT_DATA([input.m4],