Fix memory leak in regexp compiler (BZ #17069)

[BZ #17069]
	* posix/regcomp.c (parse_expression): Deallocate partially
	constructed tree before returning error.
	* posix/Makefile.c (tests): Add bug-regex36.
	(generated): Add bug-regex36.mtrace.
	(tests-special): Add $(objpfx)bug-regex36-mem.out
	(bug-regex36-ENV): New variable.
	($(objpfx)bug-regex36-mem.out): New rule.
	* posix/bug-regex36.c: New file.

Thanks, that patch looks good; please apply.

Hi Andreas,

in branch allan/2.19/backport, your patch "Fix memory leak in regexp 
compiler (BZ #17069)" (fc93c8a02c25e2486f3057ae06cf79209c381832) introduces
<<<<<<< HEAD
...
=======
...
 >>>>>>> 4d43ef1... Fix memory leak in regexp compiler (BZ #17069)
in posix/Makefile

Bye
Stefan

On 06/19/2014 04:39 PM, Andreas Schwab wrote:
> 	[BZ #17069]
> 	* posix/regcomp.c (parse_expression): Deallocate partially
> 	constructed tree before returning error.
> 	* posix/Makefile.c (tests): Add bug-regex36.
> 	(generated): Add bug-regex36.mtrace.
> 	(tests-special): Add $(objpfx)bug-regex36-mem.out
> 	(bug-regex36-ENV): New variable.
> 	($(objpfx)bug-regex36-mem.out): New rule.
> 	* posix/bug-regex36.c: New file.
>
> diff --git a/posix/Makefile b/posix/Makefile
> index 37d6d5f..e6b69b4 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -87,7 +87,7 @@ tests		:= tstgetopt testfnm runtests runptests	     \
>   		   bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
>   		   bug-getopt5 tst-getopt_long1 bug-regex34 bug-regex35 \
>   		   tst-pathconf tst-getaddrinfo4 tst-rxspencer-no-utf8 \
> -		   tst-fnmatch3
> +		   tst-fnmatch3 bug-regex36
>   xtests		:= bug-ga2
>   ifeq (yes,$(build-shared))
>   test-srcs	:= globtest
> @@ -113,7 +113,7 @@ generated += $(addprefix wordexp-test-result, 1 2 3 4 5 6 7 8 9 10) \
>   	     tst-boost.mtrace bug-ga2.mtrace bug-ga2-mem.out \
>   	     bug-glob2.mtrace bug-glob2-mem.out tst-vfork3-mem.out \
>   	     tst-vfork3.mtrace getconf.speclist tst-fnmatch-mem.out \
> -	     tst-fnmatch.mtrace
> +	     tst-fnmatch.mtrace bug-regex36.mtrace
>
>   ifeq ($(run-built-tests),yes)
>   ifeq (yes,$(build-shared))
> @@ -130,7 +130,7 @@ tests-special += $(objpfx)bug-regex2-mem.out $(objpfx)bug-regex14-mem.out \
>   		 $(objpfx)tst-rxspencer-no-utf8-mem.out $(objpfx)tst-pcre-mem.out \
>   		 $(objpfx)tst-boost-mem.out $(objpfx)tst-getconf.out \
>   		 $(objpfx)bug-glob2-mem.out $(objpfx)tst-vfork3-mem.out \
> -		 $(objpfx)tst-fnmatch-mem.out
> +		 $(objpfx)tst-fnmatch-mem.out $(objpfx)bug-regex36-mem.out
>   xtests-special += $(objpfx)bug-ga2-mem.out
>   endif
>
> @@ -260,6 +260,12 @@ $(objpfx)bug-regex31-mem.out: $(objpfx)bug-regex31.out
>   	$(common-objpfx)malloc/mtrace $(objpfx)bug-regex31.mtrace > $@; \
>   	$(evaluate-test)
>
> +bug-regex36-ENV = MALLOC_TRACE=$(objpfx)bug-regex36.mtrace
> +
> +$(objpfx)bug-regex36-mem.out: $(objpfx)bug-regex36.out
> +	$(common-objpfx)malloc/mtrace $(objpfx)bug-regex36.mtrace > $@; \
> +	$(evaluate-test)
> +
>   tst-vfork3-ENV = MALLOC_TRACE=$(objpfx)tst-vfork3.mtrace
>
>   $(objpfx)tst-vfork3-mem.out: $(objpfx)tst-vfork3.out
> diff --git a/posix/bug-regex36.c b/posix/bug-regex36.c
> new file mode 100644
> index 0000000..3dda026
> --- /dev/null
> +++ b/posix/bug-regex36.c
> @@ -0,0 +1,29 @@
> +/* Test regcomp not leaking memory on invalid repetition operator
> +   Copyright (C) 2014 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <mcheck.h>
> +#include <regex.h>
> +
> +int
> +main (int argc, char **argv)
> +{
> +  regex_t r;
> +  mtrace ();
> +  regcomp (&r, "[a]\\{-2,}", 0);
> +  regfree (&r);
> +}
> diff --git a/posix/regcomp.c b/posix/regcomp.c
> index 921d0f4..a5020be 100644
> --- a/posix/regcomp.c
> +++ b/posix/regcomp.c
> @@ -2415,14 +2415,21 @@ parse_expression (re_string_t *regexp, regex_t *preg, re_token_t *token,
>     while (token->type == OP_DUP_ASTERISK || token->type == OP_DUP_PLUS
>   	 || token->type == OP_DUP_QUESTION || token->type == OP_OPEN_DUP_NUM)
>       {
> -      tree = parse_dup_op (tree, regexp, dfa, token, syntax, err);
> -      if (BE (*err != REG_NOERROR && tree == NULL, 0))
> -	return NULL;
> +      bin_tree_t *dup_tree = parse_dup_op (tree, regexp, dfa, token, syntax, err);
> +      if (BE (*err != REG_NOERROR && dup_tree == NULL, 0))
> +	{
> +	  if (tree != NULL)
> +	    postorder (tree, free_tree, NULL);
> +	  return NULL;
> +	}
> +      tree = dup_tree;
>         /* In BRE consecutive duplications are not allowed.  */
>         if ((syntax & RE_CONTEXT_INVALID_DUP)
>   	  && (token->type == OP_DUP_ASTERISK
>   	      || token->type == OP_OPEN_DUP_NUM))
>   	{
> +	  if (tree != NULL)
> +	    postorder (tree, free_tree, NULL);
>   	  *err = REG_BADRPT;
>   	  return NULL;
>   	}
>

On 28/07/14 22:01, Stefan Liebler wrote:
> Hi Andreas,
> 
> in branch allan/2.19/backport, your patch "Fix memory leak in regexp
> compiler (BZ #17069)" (fc93c8a02c25e2486f3057ae06cf79209c381832) introduces
> <<<<<<< HEAD
> ...
> =======
> ...
>>>>>>>> 4d43ef1... Fix memory leak in regexp compiler (BZ #17069)
> in posix/Makefile
> 

Thanks for catching that.  It is a bad backport by me.

Note that I have done zero testing of that branch (clearly) which is why
I have not pushed any of it to the release/2.19/master branch yet.  I
will do so if we decide to make a glibc-2.19.1 release after reverting
the s390 mp_buf/ucontext_t ABI change.

Allan